On Thu, Oct 27, 2022 at 12:24 PM Alex Bennée <alex.ben...@linaro.org> wrote:
Hello all > Vikram Garhwal <vikram.garh...@amd.com> writes: > > > xenstore_record_dm_state() will also be used in aarch64 xenpv machine. > > > > Signed-off-by: Vikram Garhwal <vikram.garh...@amd.com> > > Signed-off-by: Stefano Stabellini <stefano.stabell...@amd.com> > > --- > > accel/xen/xen-all.c | 2 +- > > include/hw/xen/xen.h | 2 ++ > > 2 files changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/accel/xen/xen-all.c b/accel/xen/xen-all.c > > index 69aa7d018b..276625b78b 100644 > > --- a/accel/xen/xen-all.c > > +++ b/accel/xen/xen-all.c > > @@ -100,7 +100,7 @@ void xenstore_store_pv_console_info(int i, Chardev > *chr) > > } > > > > > > -static void xenstore_record_dm_state(struct xs_handle *xs, const char > *state) > > +void xenstore_record_dm_state(struct xs_handle *xs, const char *state) > > { > > char path[50]; > > > > diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h > > index afdf9c436a..31e9538a5c 100644 > > --- a/include/hw/xen/xen.h > > +++ b/include/hw/xen/xen.h > > @@ -9,6 +9,7 @@ > > */ > > > > #include "exec/cpu-common.h" > > +#include <xenstore.h> > > This is breaking a bunch of the builds and generally we try and avoid > adding system includes in headers (apart from osdep.h) for this reason. > In fact there is a comment just above to that fact. > > I think you can just add struct xs_handle to typedefs.h (or maybe just > xen.h) and directly include xenstore.h in xen-all.c following the usual > rules: > > > https://qemu.readthedocs.io/en/latest/devel/style.html#include-directives > > It might be worth doing an audit to see what else is including xen.h > needlessly or should be using sysemu/xen.h. > > > > > /* xen-machine.c */ > > enum xen_mode { > > @@ -31,5 +32,6 @@ qemu_irq *xen_interrupt_controller_init(void); > > void xenstore_store_pv_console_info(int i, Chardev *chr); > > > > void xen_register_framebuffer(struct MemoryRegion *mr); > > +void xenstore_record_dm_state(struct xs_handle *xs, const char *state); > > > > #endif /* QEMU_HW_XEN_H */ > > > -- > Alex Bennée > > For considering: I think this patch and some other changes done in "[PATCH v1 10/12] hw/arm: introduce xenpv machine" (the opening of Xen interfaces and calling xenstore_record_dm_state() in hw/arm/xen_arm.c:xen_init_ioreq()) could be avoided if we enable the Xen accelerator (either by passing "-M xenpv,accel=xen" or by adding mc->default_machine_opts = "accel=xen"; to hw/arm/xen_arm.c:xen_arm_machine_class_init() or by some other method). These actions are already done in accel/xen/xen-all.c:xen_init(). Please note, that I am not too familiar with that code, so there might be nuances. Besides that, Xen accelerator will be needed for the xen-mapcache to be in use (this is needed for mapping guest memory), there are a few xen_enabled() checks spreading around that code to perform Xen specific actions. -- Regards, Oleksandr Tyshchenko