| Message ID | 1311044012-24288-3-git-send-email-agraf@suse.de |
|---|---|
| State | New |
| Headers | show |
On 07/19/2011 05:53 AM, Alexander Graf wrote: > During the transition to get rid of CONFIG_XEN_MAPCACHE we lost the mapcache > stubs along the way. Nobody realized it because the commands were guarded by > if (xen_enabled()) clauses that made gcc optimize out the respective calls. > Except those building with --enable-debug.
On 20.07.2011, at 17:12, Avi Kivity wrote: > On 07/19/2011 05:53 AM, Alexander Graf wrote: >> During the transition to get rid of CONFIG_XEN_MAPCACHE we lost the mapcache >> stubs along the way. Nobody realized it because the commands were guarded by >> if (xen_enabled()) clauses that made gcc optimize out the respective calls. >> > > Except those building with --enable-debug. Ah, that's how you caught it. I was wondering already :). Right, --enable-debug also catches it. Alex
Am 19.07.2011 04:53, schrieb Alexander Graf: > During the transition to get rid of CONFIG_XEN_MAPCACHE we lost the > mapcache > stubs along the way. Nobody realized it because the commands were > guarded by > if (xen_enabled()) clauses that made gcc optimize out the respective > calls. > > This patch adds the stubs again - this time in the generic xen-stubs.c > > Signed-off-by: Alexander Graf <agraf@suse.de> > --- > xen-stub.c | 26 ++++++++++++++++++++++++++ > 1 files changed, 26 insertions(+), 0 deletions(-) > > diff --git a/xen-stub.c b/xen-stub.c > index efe2ab5..76d80e9 100644 > --- a/xen-stub.c > +++ b/xen-stub.c > @@ -8,6 +8,7 @@ > > #include "qemu-common.h" > #include "hw/xen.h" > +#include "xen-mapcache.h" > > void xenstore_store_pv_console_info(int i, CharDriverState *chr) > { > @@ -43,3 +44,28 @@ int xen_init(void) > { > return -ENOSYS; > } > + > +/******* mapcache stubs *******/ > + > +void xen_map_cache_init(void) > +{ > +} > + > +uint8_t *xen_map_cache(target_phys_addr_t phys_addr, > target_phys_addr_t size, > + uint8_t lock) > +{ > + return qemu_get_ram_ptr(phys_addr); > +} > + > +ram_addr_t xen_ram_addr_from_mapcache(void *ptr) > +{ > + return -1; > +} > + > +void xen_invalidate_map_cache(void) > +{ > +} > + > +void xen_invalidate_map_cache_entry(uint8_t *buffer) > +{ > +} The patch fixes a build problem, and the sooner we fix it, the better it is. Do we really need a return value other than NULL in xen_map_cache()? Do we need the stubs for xen_map_cache_init() and xen_invalidate_map_cache()? As discussed in another thread, static inline stub function might be better. Regards, Stefan W.
On 20.07.2011, at 18:41, Stefan Weil wrote: > Am 19.07.2011 04:53, schrieb Alexander Graf: >> During the transition to get rid of CONFIG_XEN_MAPCACHE we lost the mapcache >> stubs along the way. Nobody realized it because the commands were guarded by >> if (xen_enabled()) clauses that made gcc optimize out the respective calls. >> >> This patch adds the stubs again - this time in the generic xen-stubs.c >> >> Signed-off-by: Alexander Graf <agraf@suse.de> >> --- >> xen-stub.c | 26 ++++++++++++++++++++++++++ >> 1 files changed, 26 insertions(+), 0 deletions(-) >> >> diff --git a/xen-stub.c b/xen-stub.c >> index efe2ab5..76d80e9 100644 >> --- a/xen-stub.c >> +++ b/xen-stub.c >> @@ -8,6 +8,7 @@ >> >> #include "qemu-common.h" >> #include "hw/xen.h" >> +#include "xen-mapcache.h" >> >> void xenstore_store_pv_console_info(int i, CharDriverState *chr) >> { >> @@ -43,3 +44,28 @@ int xen_init(void) >> { >> return -ENOSYS; >> } >> + >> +/******* mapcache stubs *******/ >> + >> +void xen_map_cache_init(void) >> +{ >> +} >> + >> +uint8_t *xen_map_cache(target_phys_addr_t phys_addr, target_phys_addr_t size, >> + uint8_t lock) >> +{ >> + return qemu_get_ram_ptr(phys_addr); >> +} >> + >> +ram_addr_t xen_ram_addr_from_mapcache(void *ptr) >> +{ >> + return -1; >> +} >> + >> +void xen_invalidate_map_cache(void) >> +{ >> +} >> + >> +void xen_invalidate_map_cache_entry(uint8_t *buffer) >> +{ >> +} > > The patch fixes a build problem, and the sooner we fix it, > the better it is. > > Do we really need a return value other than NULL in xen_map_cache()? > Do we need the stubs for xen_map_cache_init() and xen_invalidate_map_cache()? > As discussed in another thread, static inline stub function might be better. I'm fairly sure we don't, but I merely shoved everything that was in xen-mapcache-stubs.c into xen-stubs.c. We can concentrate on optimizing this out again when the build works. If we really care about it. Alex
diff --git a/xen-stub.c b/xen-stub.c index efe2ab5..76d80e9 100644 --- a/xen-stub.c +++ b/xen-stub.c @@ -8,6 +8,7 @@ #include "qemu-common.h" #include "hw/xen.h" +#include "xen-mapcache.h" void xenstore_store_pv_console_info(int i, CharDriverState *chr) { @@ -43,3 +44,28 @@ int xen_init(void) { return -ENOSYS; } + +/******* mapcache stubs *******/ + +void xen_map_cache_init(void) +{ +} + +uint8_t *xen_map_cache(target_phys_addr_t phys_addr, target_phys_addr_t size, + uint8_t lock) +{ + return qemu_get_ram_ptr(phys_addr); +} + +ram_addr_t xen_ram_addr_from_mapcache(void *ptr) +{ + return -1; +} + +void xen_invalidate_map_cache(void) +{ +} + +void xen_invalidate_map_cache_entry(uint8_t *buffer) +{ +}
During the transition to get rid of CONFIG_XEN_MAPCACHE we lost the mapcache stubs along the way. Nobody realized it because the commands were guarded by if (xen_enabled()) clauses that made gcc optimize out the respective calls. This patch adds the stubs again - this time in the generic xen-stubs.c Signed-off-by: Alexander Graf <agraf@suse.de> --- xen-stub.c | 26 ++++++++++++++++++++++++++ 1 files changed, 26 insertions(+), 0 deletions(-)