[RFC,v2,07/22] pc_piix: handle XEN_EMULATE backend init

From: Joao Martins <joao.m.martins@oracle.com>

And use newly added xen_emulated_machine_init() to iniitalize
the xenstore and the sysdev bus for future emulated devices.

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
[dwmw2: Move it to xen-legacy-backend.c]
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 hw/i386/pc_piix.c                   |  5 +++++
 hw/xen/xen-legacy-backend.c         | 22 ++++++++++++++++------
 include/hw/xen/xen-legacy-backend.h |  2 ++
 3 files changed, 23 insertions(+), 6 deletions(-)

On 09/12/2022 09:55, David Woodhouse wrote:
> From: Joao Martins <joao.m.martins@oracle.com>
> 
> And use newly added xen_emulated_machine_init() to iniitalize
> the xenstore and the sysdev bus for future emulated devices.
> 
> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
> [dwmw2: Move it to xen-legacy-backend.c]
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> ---
>   hw/i386/pc_piix.c                   |  5 +++++
>   hw/xen/xen-legacy-backend.c         | 22 ++++++++++++++++------
>   include/hw/xen/xen-legacy-backend.h |  2 ++
>   3 files changed, 23 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 13286d0739..3dcac2f4b6 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -47,6 +47,7 @@
>   #include "hw/sysbus.h"
>   #include "hw/i2c/smbus_eeprom.h"
>   #include "hw/xen/xen-x86.h"
> +#include "hw/xen/xen-legacy-backend.h"
>   #include "exec/memory.h"
>   #include "hw/acpi/acpi.h"
>   #include "hw/acpi/piix4.h"
> @@ -155,6 +156,10 @@ static void pc_init1(MachineState *machine,
>               x86ms->above_4g_mem_size = 0;
>               x86ms->below_4g_mem_size = machine->ram_size;
>           }
> +
> +        if (pcms->xen_version && !xen_be_xenstore_open()) {

So, this is a bit subtle... it's only *because* using real Xen results 
in xen_version being 0 that this is sane? Also does this not mean that 
we are now relying on libxenstore? Shouldn't that be called out in the 
config?

> +            xen_emulated_machine_init();
> +        }
>       }
>   
>       pc_machine_init_sgx_epc(pcms);
> diff --git a/hw/xen/xen-legacy-backend.c b/hw/xen/xen-legacy-backend.c
> index 694e7bbc54..60a7bc7ab6 100644
> --- a/hw/xen/xen-legacy-backend.c
> +++ b/hw/xen/xen-legacy-backend.c
> @@ -31,6 +31,7 @@
>   #include "qapi/error.h"
>   #include "hw/xen/xen-legacy-backend.h"
>   #include "hw/xen/xen_pvdev.h"
> +#include "hw/xen/xen-bus.h"
>   #include "monitor/qdev.h"
>   
>   DeviceState *xen_sysdev;
> @@ -294,13 +295,15 @@ static struct XenLegacyDevice *xen_be_get_xendev(const char *type, int dom,
>       xendev->debug      = debug;
>       xendev->local_port = -1;
>   
> -    xendev->evtchndev = xenevtchn_open(NULL, 0);
> -    if (xendev->evtchndev == NULL) {
> -        xen_pv_printf(NULL, 0, "can't open evtchn device\n");
> -        qdev_unplug(DEVICE(xendev), NULL);
> -        return NULL;
> +    if (xen_mode != XEN_EMULATE) {
> +        xendev->evtchndev = xenevtchn_open(NULL, 0);

Doesn't this need stubbing out so that we can build without libxenevtchn?

   Paul

> +        if (xendev->evtchndev == NULL) {
> +            xen_pv_printf(NULL, 0, "can't open evtchn device\n");
> +            qdev_unplug(DEVICE(xendev), NULL);
> +            return NULL;
> +        }
> +        qemu_set_cloexec(xenevtchn_fd(xendev->evtchndev));
>       }
> -    qemu_set_cloexec(xenevtchn_fd(xendev->evtchndev));
>   
>       xen_pv_insert_xendev(xendev);
>   
> @@ -859,3 +862,10 @@ static void xenbe_register_types(void)
>   }
>   
>   type_init(xenbe_register_types)
> +
> +void xen_emulated_machine_init(void)
> +{
> +    xen_bus_init();
> +    xen_be_sysdev_init();
> +    xen_be_register_common();
> +}
> diff --git a/include/hw/xen/xen-legacy-backend.h b/include/hw/xen/xen-legacy-backend.h
> index 0aa171f6c2..aa09015662 100644
> --- a/include/hw/xen/xen-legacy-backend.h
> +++ b/include/hw/xen/xen-legacy-backend.h
> @@ -105,4 +105,6 @@ int xen_config_dev_vfb(int vdev, const char *type);
>   int xen_config_dev_vkbd(int vdev);
>   int xen_config_dev_console(int vdev);
>   
> +void xen_emulated_machine_init(void);
> +
>   #endif /* HW_XEN_LEGACY_BACKEND_H */

On Mon, 2022-12-12 at 13:47 +0000, Paul Durrant wrote:
> 
> > @@ -155,6 +156,10 @@ static void pc_init1(MachineState *machine,
> >                x86ms->above_4g_mem_size = 0;
> >                x86ms->below_4g_mem_size = machine->ram_size;
> >            }
> > +
> > +        if (pcms->xen_version && !xen_be_xenstore_open()) {
> 
> So, this is a bit subtle... it's only *because* using real Xen results 
> in xen_version being 0 that this is sane? Also does this not mean that 
> we are now relying on libxenstore? Shouldn't that be called out in the 
> config?

None of the CONFIG_XENFV_MACHINE code builds right now unless
CONFIG_XEN is set anyway. We can move code around and use #ifdef
appropriately once the dust has settled on how the config options are
going to relate to one another; doing that too soon seemed like
pointless churn. I know I didn't *quite* do the config options the way
that Philippe said, so figured it was better to wait until we have
consensus.

As noted in the cover letter, "For now, we just need to be able to use
the xenfv machine in order to instantiate the shinfo and evtchn
objects."

So for now I've basically just stuck with what was in the original
patchset, and this is going to change.

Ideally, I'd like to avoid the external xenstore completely. We could
have a completely internal implementation which is private to the
guest. Since this isn't true Xen, the guest has no way of talking to
anything other than qemu itself, which will play the rôle of dom0.