Re: [PATCH v6 08/51] xen-platform: allow its creation with XEN_EMULATE mode

[Paul Durrant]

On 10/01/2023 12:19, David Woodhouse wrote:
> From: Joao Martins <joao.m.martins@oracle.com>
>
> The only thing we need to handle on KVM side is to change the
> pfn from R/W to R/O.
>
> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> ---
>   hw/i386/xen/meson.build    |  5 ++++-
>   hw/i386/xen/xen_platform.c | 39 +++++++++++++++++++++++++-------------
>   2 files changed, 30 insertions(+), 14 deletions(-)
>
> diff --git a/hw/i386/xen/meson.build b/hw/i386/xen/meson.build
> index be84130300..79d75cc927 100644
> --- a/hw/i386/xen/meson.build
> +++ b/hw/i386/xen/meson.build
> @@ -2,6 +2,9 @@ i386_ss.add(when: 'CONFIG_XEN', if_true: files(
>     'xen-hvm.c',
>     'xen-mapcache.c',
>     'xen_apic.c',
> -  'xen_platform.c',
>     'xen_pvdevice.c',
>   ))
> +
> +i386_ss.add(when: 'CONFIG_XENFV_MACHINE', if_true: files(
> +  'xen_platform.c',
> +))
> diff --git a/hw/i386/xen/xen_platform.c b/hw/i386/xen/xen_platform.c
> index 50174c2269..00f0527b30 100644
> --- a/hw/i386/xen/xen_platform.c
> +++ b/hw/i386/xen/xen_platform.c
> @@ -28,9 +28,9 @@
>   #include "hw/ide.h"
>   #include "hw/ide/pci.h"
>   #include "hw/pci/pci.h"
> -#include "hw/xen/xen_common.h"
>   #include "migration/vmstate.h"
> -#include "hw/xen/xen-legacy-backend.h"
> +#include "hw/xen/xen.h"
> +#include "net/net.h"
>   #include "trace.h"
>   #include "sysemu/xen.h"
>   #include "sysemu/block-backend.h"
> @@ -38,6 +38,11 @@
>   #include "qemu/module.h"
>   #include "qom/object.h"
>   
> +#ifdef CONFIG_XEN
> +#include "hw/xen/xen_common.h"
> +#include "hw/xen/xen-legacy-backend.h"
> +#endif
> +
>   //#define DEBUG_PLATFORM
>   
>   #ifdef DEBUG_PLATFORM
> @@ -280,18 +285,26 @@ static void platform_fixed_ioport_writeb(void *opaque, 
> uint32_t addr, uint32_t v
>       PCIXenPlatformState *s = opaque;
>   
>       switch (addr) {
> -    case 0: /* Platform flags */ {
> -        hvmmem_type_t mem_type = (val & PFFLAG_ROM_LOCK) ?
> -            HVMMEM_ram_ro : HVMMEM_ram_rw;
> -        if (xen_set_mem_type(xen_domid, mem_type, 0xc0, 0x40)) {
> -            DPRINTF("unable to change ro/rw state of ROM memory area!\n");
> -        } else {
> +    case 0: /* Platform flags */
> +        if (xen_mode == XEN_EMULATE) {
> +            /* XX: Use i440gx/q35 PAM setup to do this? */
>               s->flags = val & PFFLAG_ROM_LOCK;

Given that this is not RFC, do you have a definite plan? TBH I think 
only ancient (Bochs) ROMBIOS messes with this; I can't find any trace in 
SeaBIOS anyway. So maybe we just don't care.

  Paul

> -            DPRINTF("changed ro/rw state of ROM memory area. now is %s 
> state.\n",
> -                    (mem_type == HVMMEM_ram_ro ? "ro":"rw"));
> +#ifdef CONFIG_XEN
> +        } else {
> +            hvmmem_type_t mem_type = (val & PFFLAG_ROM_LOCK) ?
> +                HVMMEM_ram_ro : HVMMEM_ram_rw;
> +
> +            if (xen_set_mem_type(xen_domid, mem_type, 0xc0, 0x40)) {
> +                DPRINTF("unable to change ro/rw state of ROM memory area!\n");
> +            } else {
> +                s->flags = val & PFFLAG_ROM_LOCK;
> +                DPRINTF("changed ro/rw state of ROM memory area. now is %s 
> state.\n",
> +                        (mem_type == HVMMEM_ram_ro ? "ro" : "rw"));
> +            }
> +#endif
>           }
>           break;
> -    }
> +
>       case 2:
>           log_writeb(s, val);
>           break;
> @@ -509,8 +522,8 @@ static void xen_platform_realize(PCIDevice *dev, Error 
> **errp)
>       uint8_t *pci_conf;
>   
>       /* Device will crash on reset if xen is not initialized */
> -    if (!xen_enabled()) {
> -        error_setg(errp, "xen-platform device requires the Xen accelerator");
> +    if (xen_mode == XEN_DISABLED) {
> +        error_setg(errp, "xen-platform device requires a Xen guest");
>           return;
>       }
>