| Message ID | 20221209095612.689243-6-dwmw2@infradead.org |
|---|---|
| State | New |
| Headers | show |
| Series | Xen HVM support under KVM | expand |
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);
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 --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);