diff mbox series

[RFC,v2,05/22] xen-platform-pci: allow its creation with XEN_EMULATE mode

Message ID 20221209095612.689243-6-dwmw2@infradead.org
State New
Headers show
Series Xen HVM support under KVM | expand

Commit Message

David Woodhouse Dec. 9, 2022, 9:55 a.m. UTC
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/xen_platform.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

Comments

Paul Durrant Dec. 12, 2022, 1:24 p.m. UTC | #1
On 09/12/2022 09:55, 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/xen_platform.c | 11 ++++-------
>   1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/i386/xen/xen_platform.c b/hw/i386/xen/xen_platform.c
> index a64265cca0..914619d140 100644
> --- a/hw/i386/xen/xen_platform.c
> +++ b/hw/i386/xen/xen_platform.c
> @@ -271,7 +271,10 @@ static void platform_fixed_ioport_writeb(void *opaque, uint32_t addr, uint32_t v
>       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)) {
> +        if (xen_mode == XEN_EMULATE) {
> +            /* XXX */
> +            s->flags = val & PFFLAG_ROM_LOCK;
> +        } else 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;

Surely this would cleaner as:

if (xen_mode != XEN_EMULATE && 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;

?

   Paul

> @@ -496,12 +499,6 @@ static void xen_platform_realize(PCIDevice *dev, Error **errp)
>       PCIXenPlatformState *d = XEN_PLATFORM(dev);
>       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");
> -        return;
> -    }
> -
>       pci_conf = dev->config;
>   
>       pci_set_word(pci_conf + PCI_COMMAND, PCI_COMMAND_IO | PCI_COMMAND_MEMORY);
David Woodhouse Dec. 12, 2022, 10:07 p.m. UTC | #2
On Mon, 2022-12-12 at 13:24 +0000, Paul Durrant wrote:
> On 09/12/2022 09:55, David Woodhouse wrote:
> > --- a/hw/i386/xen/xen_platform.c
> > +++ b/hw/i386/xen/xen_platform.c
> > @@ -271,7 +271,10 @@ static void platform_fixed_ioport_writeb(void *opaque, uint32_t addr, uint32_t v
> >        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)) {
> > +        if (xen_mode == XEN_EMULATE) {
> > +            /* XXX */
> > +            s->flags = val & PFFLAG_ROM_LOCK;
> > +        } else 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;
> 
> 
> 
> Surely this would cleaner as:
> 
> 
> 
> if (xen_mode != XEN_EMULATE && 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;

Or maybe it should actually call into the PIIX code for frobbing the
read-only state of the UMBs? Do we even implement that in qemu? But
again, this part is just the necessary evil to make the thing testable
with -M xenfv for now.

I'm going to take a closer look at Paolo's suggestion which should
reduce the amount of such noise before we get to the real parts.
diff mbox series

Patch

diff --git a/hw/i386/xen/xen_platform.c b/hw/i386/xen/xen_platform.c
index a64265cca0..914619d140 100644
--- a/hw/i386/xen/xen_platform.c
+++ b/hw/i386/xen/xen_platform.c
@@ -271,7 +271,10 @@  static void platform_fixed_ioport_writeb(void *opaque, uint32_t addr, uint32_t v
     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)) {
+        if (xen_mode == XEN_EMULATE) {
+            /* XXX */
+            s->flags = val & PFFLAG_ROM_LOCK;
+        } else 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;
@@ -496,12 +499,6 @@  static void xen_platform_realize(PCIDevice *dev, Error **errp)
     PCIXenPlatformState *d = XEN_PLATFORM(dev);
     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");
-        return;
-    }
-
     pci_conf = dev->config;
 
     pci_set_word(pci_conf + PCI_COMMAND, PCI_COMMAND_IO | PCI_COMMAND_MEMORY);