| Message ID | 20250422192819.302784-14-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | single-binary patch queue | expand |
Hi Pierrick, On 22/4/25 21:26, Richard Henderson wrote: > From: Pierrick Bouvier <pierrick.bouvier@linaro.org> > > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> > Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> > Reviewed-by: Richard Henderson <richard.henderson@linaro.org> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > Message-ID: <20250317183417.285700-14-pierrick.bouvier@linaro.org> > --- > include/system/xen-mapcache.h | 41 ----------------------------------- > include/system/xen.h | 21 +++--------------- > 2 files changed, 3 insertions(+), 59 deletions(-) > diff --git a/include/system/xen.h b/include/system/xen.h > index 990c19a8ef..5f41915732 100644 > --- a/include/system/xen.h > +++ b/include/system/xen.h > @@ -25,30 +25,15 @@ > #endif /* COMPILING_PER_TARGET */ > > #ifdef CONFIG_XEN_IS_POSSIBLE > - > extern bool xen_allowed; > - > #define xen_enabled() (xen_allowed) > +#else /* !CONFIG_XEN_IS_POSSIBLE */ > +#define xen_enabled() 0 > +#endif /* CONFIG_XEN_IS_POSSIBLE */ Just to be sure, you said we should remove CONFIG_XEN_IS_POSSIBLE?
On 4/23/25 02:22, Philippe Mathieu-Daudé wrote: > Hi Pierrick, > > On 22/4/25 21:26, Richard Henderson wrote: >> From: Pierrick Bouvier <pierrick.bouvier@linaro.org> >> >> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> >> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> >> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >> Message-ID: <20250317183417.285700-14-pierrick.bouvier@linaro.org> >> --- >> include/system/xen-mapcache.h | 41 ----------------------------------- >> include/system/xen.h | 21 +++--------------- >> 2 files changed, 3 insertions(+), 59 deletions(-) > > >> diff --git a/include/system/xen.h b/include/system/xen.h >> index 990c19a8ef..5f41915732 100644 >> --- a/include/system/xen.h >> +++ b/include/system/xen.h >> @@ -25,30 +25,15 @@ >> #endif /* COMPILING_PER_TARGET */ >> >> #ifdef CONFIG_XEN_IS_POSSIBLE >> - >> extern bool xen_allowed; >> - >> #define xen_enabled() (xen_allowed) >> +#else /* !CONFIG_XEN_IS_POSSIBLE */ >> +#define xen_enabled() 0 >> +#endif /* CONFIG_XEN_IS_POSSIBLE */ > > Just to be sure, you said we should remove CONFIG_XEN_IS_POSSIBLE? More "Ideally, it should not have been introduced", and {accel}_enabled should be a proper function, and needed stubs should be added for other functions. CONFIG_{ACCEL}_IS_POSSIBLE is just "yet another way" to stub some accelerator functions, while we could have proper stubs instead. As long as it's not blocking our work, I don't think we should do this cleanup. For your case, concerning hvf, I said it would be preferable to simply implement hvf_enabled() as normal function, instead of mimicking the CONFIG_{ACCEL}_IS_POSSIBLE pattern, since it has not yet been introduced for this accelerator. It it's more simple to simply replicate this, no worries, go ahead, it just concerns one header. When we'll want to link the single binary for the first time, we'll have to deal with that anyway, as it won't be possible to have two different definition of a single function/symbol (i.e. unify stubs and implementations). But we are not yet there, and it's better to focus our energy on removing compilation units duplication at the moment.
diff --git a/include/system/xen-mapcache.h b/include/system/xen-mapcache.h index b68f196ddd..bb454a7c96 100644 --- a/include/system/xen-mapcache.h +++ b/include/system/xen-mapcache.h @@ -14,8 +14,6 @@ typedef hwaddr (*phys_offset_to_gaddr_t)(hwaddr phys_offset, ram_addr_t size); -#ifdef CONFIG_XEN_IS_POSSIBLE - void xen_map_cache_init(phys_offset_to_gaddr_t f, void *opaque); uint8_t *xen_map_cache(MemoryRegion *mr, hwaddr phys_addr, hwaddr size, @@ -28,44 +26,5 @@ void xen_invalidate_map_cache(void); uint8_t *xen_replace_cache_entry(hwaddr old_phys_addr, hwaddr new_phys_addr, hwaddr size); -#else - -static inline void xen_map_cache_init(phys_offset_to_gaddr_t f, - void *opaque) -{ -} - -static inline uint8_t *xen_map_cache(MemoryRegion *mr, - hwaddr phys_addr, - hwaddr size, - ram_addr_t ram_addr_offset, - uint8_t lock, - bool dma, - bool is_write) -{ - abort(); -} - -static inline ram_addr_t xen_ram_addr_from_mapcache(void *ptr) -{ - abort(); -} - -static inline void xen_invalidate_map_cache_entry(uint8_t *buffer) -{ -} - -static inline void xen_invalidate_map_cache(void) -{ -} - -static inline uint8_t *xen_replace_cache_entry(hwaddr old_phys_addr, - hwaddr new_phys_addr, - hwaddr size) -{ - abort(); -} - -#endif #endif /* XEN_MAPCACHE_H */ diff --git a/include/system/xen.h b/include/system/xen.h index 990c19a8ef..5f41915732 100644 --- a/include/system/xen.h +++ b/include/system/xen.h @@ -25,30 +25,15 @@ #endif /* COMPILING_PER_TARGET */ #ifdef CONFIG_XEN_IS_POSSIBLE - extern bool xen_allowed; - #define xen_enabled() (xen_allowed) +#else /* !CONFIG_XEN_IS_POSSIBLE */ +#define xen_enabled() 0 +#endif /* CONFIG_XEN_IS_POSSIBLE */ void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length); void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, struct MemoryRegion *mr, Error **errp); - -#else /* !CONFIG_XEN_IS_POSSIBLE */ - -#define xen_enabled() 0 -static inline void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length) -{ - /* nothing */ -} -static inline void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, - MemoryRegion *mr, Error **errp) -{ - g_assert_not_reached(); -} - -#endif /* CONFIG_XEN_IS_POSSIBLE */ - bool xen_mr_is_memory(MemoryRegion *mr); bool xen_mr_is_grants(MemoryRegion *mr); #endif