[Qemu-devel] [PATCH 0/2] ppc: Automatically set vga framebuffer endianness based on guest

[Qemu-devel] [PATCH 0/2] ppc: Automatically set vga framebuffer endianness based on guest

[David Gibson]

This is a reworked version of BenH's patch to set VGA framebuffer
endianess based on the endianness of the guest (as determined by the
H_SET_MODE hypercall).

I've split the patch into two parts the first adding the object
property to the VGA device allowing control of the endianness, and the
second manipulating it from the pseries code.

David Gibson (2):
  vga: Expose framebuffer byteorder as a QOM property
  pseries: Switch VGA endian on H_SET_MODE

 hw/display/vga-pci.c   | 22 ++++++++++++++++++++++
 hw/ppc/spapr_hcall.c   |  2 ++
 hw/ppc/spapr_pci.c     | 28 ++++++++++++++++++++++++++++
 include/hw/ppc/spapr.h |  1 +
 4 files changed, 53 insertions(+)

-- 
2.1.0

[Qemu-devel] [PATCH 1/2] vga: Expose framebuffer byteorder as a QOM property

[David Gibson]

The VGA device model now supports having the framebuffer in either endian,
and can be switched between these by the guest via a register in the qext
region.

However, in some cases (e.g. LE OS on the pseries machine) we have
existing guest that don't know about the endian switch register, but other
parts of the qemu code have better information to set a default endianness
than the VGA code does of itself.

In order to allow them to set a correct default endianness in these cases,
without breaking abstraction walls, this patch exposes the VGA framebuffer
endianness via a writable QOM property.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

prop fixup
---
 hw/display/vga-pci.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/hw/display/vga-pci.c b/hw/display/vga-pci.c
index 53739e4..8bd6ff7 100644
--- a/hw/display/vga-pci.c
+++ b/hw/display/vga-pci.c
@@ -181,6 +181,20 @@ static void pci_vga_qext_write(void *ptr, hwaddr addr,
     }
 }
 
+static bool vga_get_big_endian_fb(Object *obj, Error **errp)
+{
+    PCIVGAState *d = DO_UPCAST(PCIVGAState, dev, PCI_DEVICE(obj));
+
+    return d->vga.big_endian_fb;
+}
+
+static void vga_set_big_endian_fb(Object *obj, bool value, Error **errp)
+{
+    PCIVGAState *d = DO_UPCAST(PCIVGAState, dev, PCI_DEVICE(obj));
+
+    d->vga.big_endian_fb = value;
+}
+
 static const MemoryRegionOps pci_vga_qext_ops = {
     .read = pci_vga_qext_read,
     .write = pci_vga_qext_write,
@@ -233,6 +247,10 @@ static int pci_std_vga_initfn(PCIDevice *dev)
         vga_init_vbe(s, OBJECT(dev), pci_address_space(dev));
     }
 
+    /* Expose framebuffer byteorder via QOM */
+    object_property_add_bool(OBJECT(dev), "big-endian-framebuffer",
+                             vga_get_big_endian_fb, vga_set_big_endian_fb, NULL);
+
     return 0;
 }
 
@@ -268,6 +286,10 @@ static int pci_secondary_vga_initfn(PCIDevice *dev)
     pci_register_bar(&d->dev, 0, PCI_BASE_ADDRESS_MEM_PREFETCH, &s->vram);
     pci_register_bar(&d->dev, 2, PCI_BASE_ADDRESS_SPACE_MEMORY, &d->mmio);
 
+    /* Expose framebuffer byteorder via QOM */
+    object_property_add_bool(OBJECT(dev), "big-endian-framebuffer",
+                             vga_get_big_endian_fb, vga_set_big_endian_fb, NULL);
+
     return 0;
 }
 
-- 
2.1.0

Re: [Qemu-devel] [PATCH 1/2] vga: Expose framebuffer byteorder as a QOM property

[Gerd Hoffmann]

On Di, 2015-02-10 at 15:36 +1100, David Gibson wrote:
> The VGA device model now supports having the framebuffer in either endian,
> and can be switched between these by the guest via a register in the qext
> region.
> 
> However, in some cases (e.g. LE OS on the pseries machine) we have
> existing guest that don't know about the endian switch register, but other
> parts of the qemu code have better information to set a default endianness
> than the VGA code does of itself.
> 
> In order to allow them to set a correct default endianness in these cases,
> without breaking abstraction walls, this patch exposes the VGA framebuffer
> endianness via a writable QOM property.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> 
> prop fixup

squash leftover?

Otherwise:

Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>

(assuming this little series gets in via ppc tree).

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 1/2] vga: Expose framebuffer byteorder as a QOM property

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1268 bytes --]

On Tue, Feb 10, 2015 at 10:19:58AM +0100, Gerd Hoffmann wrote:
> On Di, 2015-02-10 at 15:36 +1100, David Gibson wrote:
> > The VGA device model now supports having the framebuffer in either endian,
> > and can be switched between these by the guest via a register in the qext
> > region.
> > 
> > However, in some cases (e.g. LE OS on the pseries machine) we have
> > existing guest that don't know about the endian switch register, but other
> > parts of the qemu code have better information to set a default endianness
> > than the VGA code does of itself.
> > 
> > In order to allow them to set a correct default endianness in these cases,
> > without breaking abstraction walls, this patch exposes the VGA framebuffer
> > endianness via a writable QOM property.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > 
> > prop fixup
> 
> squash leftover?

Oops, yes.

> Otherwise:
> 
> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
> 
> (assuming this little series gets in via ppc tree).
> 
> cheers,
>   Gerd
> 
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

[Qemu-devel] [PATCH 2/2] pseries: Switch VGA endian on H_SET_MODE

[David Gibson]

When the guest switches the interrupt endian mode, which essentially
means a global machine endian switch, we want to change the VGA
framebuffer endian mode as well in order to be backward compatible
with existing guests who don't know about the new endian control
register.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr_hcall.c   |  2 ++
 hw/ppc/spapr_pci.c     | 28 ++++++++++++++++++++++++++++
 include/hw/ppc/spapr.h |  1 +
 3 files changed, 31 insertions(+)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 8651447..4f76f1c 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -731,12 +731,14 @@ static target_ulong h_set_mode_resource_le(PowerPCCPU *cpu,
         CPU_FOREACH(cs) {
             set_spr(cs, SPR_LPCR, 0, LPCR_ILE);
         }
+        spapr_pci_switch_vga(true);
         return H_SUCCESS;
 
     case H_SET_MODE_ENDIAN_LITTLE:
         CPU_FOREACH(cs) {
             set_spr(cs, SPR_LPCR, LPCR_ILE, LPCR_ILE);
         }
+        spapr_pci_switch_vga(false);
         return H_SUCCESS;
     }
 
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 21b95b3..acf9e82 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -966,3 +966,31 @@ static void spapr_pci_register_types(void)
 }
 
 type_init(spapr_pci_register_types)
+
+static int spapr_switch_one_vga(DeviceState *dev, void *opaque)
+{
+    bool be = *(bool *)opaque;
+
+    if (object_dynamic_cast(OBJECT(dev), "VGA")
+        || object_dynamic_cast(OBJECT(dev), "secondary-vga")) {
+        object_property_set_bool(OBJECT(dev), be, "big-endian-framebuffer",
+                                 &error_abort);
+    }
+    return 0;
+}
+
+void spapr_pci_switch_vga(bool big_endian)
+{
+    sPAPRPHBState *sphb;
+
+    /*
+     * For backward compatibility with existing guests, we switch
+     * the endianness of the VGA controller when changing the guest
+     * interrupt mode
+     */
+    QLIST_FOREACH(sphb, &spapr->phbs, list) {
+        BusState *bus = &PCI_HOST_BRIDGE(sphb)->bus->qbus;
+        qbus_walk_children(bus, spapr_switch_one_vga, NULL, NULL, NULL,
+                           &big_endian);
+    }
+}
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 716bff4..6aadc94 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -480,5 +480,6 @@ int spapr_dma_dt(void *fdt, int node_off, const char *propname,
                  uint32_t liobn, uint64_t window, uint32_t size);
 int spapr_tcet_dma_dt(void *fdt, int node_off, const char *propname,
                       sPAPRTCETable *tcet);
+void spapr_pci_switch_vga(bool big_endian);
 
 #endif /* !defined (__HW_SPAPR_H__) */
-- 
2.1.0

Re: [Qemu-devel] [PATCH 2/2] pseries: Switch VGA endian on H_SET_MODE

[Michael Roth]

Quoting David Gibson (2015-02-09 22:36:16)
> When the guest switches the interrupt endian mode, which essentially
> means a global machine endian switch, we want to change the VGA
> framebuffer endian mode as well in order to be backward compatible
> with existing guests who don't know about the new endian control
> register.
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

I think maybe Gerd's review applied to the whole series, but:

Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>

> ---
>  hw/ppc/spapr_hcall.c   |  2 ++
>  hw/ppc/spapr_pci.c     | 28 ++++++++++++++++++++++++++++
>  include/hw/ppc/spapr.h |  1 +
>  3 files changed, 31 insertions(+)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 8651447..4f76f1c 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -731,12 +731,14 @@ static target_ulong h_set_mode_resource_le(PowerPCCPU *cpu,
>          CPU_FOREACH(cs) {
>              set_spr(cs, SPR_LPCR, 0, LPCR_ILE);
>          }
> +        spapr_pci_switch_vga(true);
>          return H_SUCCESS;
> 
>      case H_SET_MODE_ENDIAN_LITTLE:
>          CPU_FOREACH(cs) {
>              set_spr(cs, SPR_LPCR, LPCR_ILE, LPCR_ILE);
>          }
> +        spapr_pci_switch_vga(false);
>          return H_SUCCESS;
>      }
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 21b95b3..acf9e82 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -966,3 +966,31 @@ static void spapr_pci_register_types(void)
>  }
> 
>  type_init(spapr_pci_register_types)
> +
> +static int spapr_switch_one_vga(DeviceState *dev, void *opaque)
> +{
> +    bool be = *(bool *)opaque;
> +
> +    if (object_dynamic_cast(OBJECT(dev), "VGA")
> +        || object_dynamic_cast(OBJECT(dev), "secondary-vga")) {
> +        object_property_set_bool(OBJECT(dev), be, "big-endian-framebuffer",
> +                                 &error_abort);
> +    }
> +    return 0;
> +}
> +
> +void spapr_pci_switch_vga(bool big_endian)
> +{
> +    sPAPRPHBState *sphb;
> +
> +    /*
> +     * For backward compatibility with existing guests, we switch
> +     * the endianness of the VGA controller when changing the guest
> +     * interrupt mode
> +     */
> +    QLIST_FOREACH(sphb, &spapr->phbs, list) {
> +        BusState *bus = &PCI_HOST_BRIDGE(sphb)->bus->qbus;
> +        qbus_walk_children(bus, spapr_switch_one_vga, NULL, NULL, NULL,
> +                           &big_endian);
> +    }
> +}
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 716bff4..6aadc94 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -480,5 +480,6 @@ int spapr_dma_dt(void *fdt, int node_off, const char *propname,
>                   uint32_t liobn, uint64_t window, uint32_t size);
>  int spapr_tcet_dma_dt(void *fdt, int node_off, const char *propname,
>                        sPAPRTCETable *tcet);
> +void spapr_pci_switch_vga(bool big_endian);
> 
>  #endif /* !defined (__HW_SPAPR_H__) */
> -- 
> 2.1.0

Re: [Qemu-devel] [PATCH 0/2] ppc: Automatically set vga framebuffer endianness based on guest

[Alexander Graf]

On 10.02.15 05:36, David Gibson wrote:
> This is a reworked version of BenH's patch to set VGA framebuffer
> endianess based on the endianness of the guest (as determined by the
> H_SET_MODE hypercall).
> 
> I've split the patch into two parts the first adding the object
> property to the VGA device allowing control of the endianness, and the
> second manipulating it from the pseries code.
> 
> David Gibson (2):
>   vga: Expose framebuffer byteorder as a QOM property
>   pseries: Switch VGA endian on H_SET_MODE

Thanks, applied all to ppc-next.


Alex