[Qemu-devel] [PATCH 0/3] macppc: minor patches for 4.0

[Qemu-devel] [PATCH 0/3] macppc: minor patches for 4.0

[Mark Cave-Ayland]

Here are a few minor macppc patches that have been sitting in my local queue
during freeze in preparation for 4.0.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


Mark Cave-Ayland (3):
  MAINTAINERS: add myself as maintainer for Mac Old World and New World
    machines
  mac_newworld: enable access to EDID data for the VGA device
  mac_oldworld: enable access to EDID data for the VGA device

 MAINTAINERS           | 6 ++++--
 hw/ppc/mac_newworld.c | 5 ++++-
 hw/ppc/mac_oldworld.c | 5 ++++-
 3 files changed, 12 insertions(+), 4 deletions(-)

-- 
2.11.0

[Qemu-devel] [PATCH 1/3] MAINTAINERS: add myself as maintainer for Mac Old World and New World machines

[Mark Cave-Ayland]

I've unofficially been doing most of the work on the Mac machines for a while
now, so update MAINTAINERS to reflect this. David is still happy to be listed
as a reviewer as per our discussion at KVM forum.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 MAINTAINERS | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 63effdc473..64bffaecca 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -935,7 +935,8 @@ F: hw/ppc/mpc8544ds.c
 F: hw/ppc/mpc8544_guts.c
 
 New World
-M: David Gibson <david@gibson.dropbear.id.au>
+M: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
+R: David Gibson <david@gibson.dropbear.id.au>
 L: qemu-ppc@nongnu.org
 S: Odd Fixes
 F: hw/ppc/mac_newworld.c
@@ -949,7 +950,8 @@ F: include/hw/misc/mos6522.h
 F: include/hw/ppc/mac_dbdma.h
 
 Old World
-M: David Gibson <david@gibson.dropbear.id.au>
+M: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
+R: David Gibson <david@gibson.dropbear.id.au>
 L: qemu-ppc@nongnu.org
 S: Odd Fixes
 F: hw/ppc/mac_oldworld.c
-- 
2.11.0

Re: [Qemu-devel] [PATCH 1/3] MAINTAINERS: add myself as maintainer for Mac Old World and New World machines

[David Gibson]

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

On Fri, Dec 07, 2018 at 04:08:04PM +0000, Mark Cave-Ayland wrote:
> I've unofficially been doing most of the work on the Mac machines for a while
> now, so update MAINTAINERS to reflect this. David is still happy to be listed
> as a reviewer as per our discussion at KVM forum.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

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

> ---
>  MAINTAINERS | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 63effdc473..64bffaecca 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -935,7 +935,8 @@ F: hw/ppc/mpc8544ds.c
>  F: hw/ppc/mpc8544_guts.c
>  
>  New World
> -M: David Gibson <david@gibson.dropbear.id.au>
> +M: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> +R: David Gibson <david@gibson.dropbear.id.au>
>  L: qemu-ppc@nongnu.org
>  S: Odd Fixes
>  F: hw/ppc/mac_newworld.c
> @@ -949,7 +950,8 @@ F: include/hw/misc/mos6522.h
>  F: include/hw/ppc/mac_dbdma.h
>  
>  Old World
> -M: David Gibson <david@gibson.dropbear.id.au>
> +M: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> +R: David Gibson <david@gibson.dropbear.id.au>
>  L: qemu-ppc@nongnu.org
>  S: Odd Fixes
>  F: hw/ppc/mac_oldworld.c

-- 
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: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[Qemu-devel] [PATCH 2/3] mac_newworld: enable access to EDID data for the VGA device

[Mark Cave-Ayland]

This is in preparation for some upcoming QEMU NDRV driver changes that pass
display information from the host to the guest.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/ppc/mac_newworld.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 14273a123e..df0a2f03ff 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -430,7 +430,10 @@ static void ppc_core99_init(MachineState *machine)
         }
     }
 
-    pci_vga_init(pci_bus);
+    dev = qdev_create(BUS(pci_bus), "VGA");
+    qdev_prop_set_int32(dev, "addr", -1);
+    qdev_prop_set_bit(dev, "edid", true);
+    qdev_init_nofail(dev);
 
     if (graphic_depth != 15 && graphic_depth != 32 && graphic_depth != 8) {
         graphic_depth = 15;
-- 
2.11.0

Re: [Qemu-devel] [PATCH 2/3] mac_newworld: enable access to EDID data for the VGA device

[David Gibson]

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

On Fri, Dec 07, 2018 at 04:08:05PM +0000, Mark Cave-Ayland wrote:
> This is in preparation for some upcoming QEMU NDRV driver changes that pass
> display information from the host to the guest.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

This looks fine by my limited knowledge of this area.  I'm slightly
perturbed I can't see any existing examples in the tree of setting the
edid property from the machine.

> ---
>  hw/ppc/mac_newworld.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
> index 14273a123e..df0a2f03ff 100644
> --- a/hw/ppc/mac_newworld.c
> +++ b/hw/ppc/mac_newworld.c
> @@ -430,7 +430,10 @@ static void ppc_core99_init(MachineState *machine)
>          }
>      }
>  
> -    pci_vga_init(pci_bus);
> +    dev = qdev_create(BUS(pci_bus), "VGA");
> +    qdev_prop_set_int32(dev, "addr", -1);
> +    qdev_prop_set_bit(dev, "edid", true);
> +    qdev_init_nofail(dev);
>  
>      if (graphic_depth != 15 && graphic_depth != 32 && graphic_depth != 8) {
>          graphic_depth = 15;

-- 
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: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Qemu-devel] [PATCH 2/3] mac_newworld: enable access to EDID data for the VGA device

[Mark Cave-Ayland]

On 10/12/2018 03:46, David Gibson wrote:

> On Fri, Dec 07, 2018 at 04:08:05PM +0000, Mark Cave-Ayland wrote:
>> This is in preparation for some upcoming QEMU NDRV driver changes that pass
>> display information from the host to the guest.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> 
> This looks fine by my limited knowledge of this area.  I'm slightly
> perturbed I can't see any existing examples in the tree of setting the
> edid property from the machine.

Certainly both patches work in that the corresponding MemoryRegion appears in both
machines when applied, and indeed now also I have guest code that can read from it too.


ATB,

Mark.

Re: [Qemu-devel] [PATCH 2/3] mac_newworld: enable access to EDID data for the VGA device

[Gerd Hoffmann]

On Fri, Dec 07, 2018 at 04:08:05PM +0000, Mark Cave-Ayland wrote:
> This is in preparation for some upcoming QEMU NDRV driver changes that pass
> display information from the host to the guest.

> -    pci_vga_init(pci_bus);
> +    dev = qdev_create(BUS(pci_bus), "VGA");
> +    qdev_prop_set_int32(dev, "addr", -1);
> +    qdev_prop_set_bit(dev, "edid", true);
> +    qdev_init_nofail(dev);

Hmm.  IMO you should not overwrite the device defaults here.

edid is off by default only because it is new and I'm conservative.
I want a release (or two) with it being available for user testing.
If no issues pop up flip it to default on.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 2/3] mac_newworld: enable access to EDID data for the VGA device

[Mark Cave-Ayland]

On 12/12/2018 08:32, Gerd Hoffmann wrote:

> On Fri, Dec 07, 2018 at 04:08:05PM +0000, Mark Cave-Ayland wrote:
>> This is in preparation for some upcoming QEMU NDRV driver changes that pass
>> display information from the host to the guest.
> 
>> -    pci_vga_init(pci_bus);
>> +    dev = qdev_create(BUS(pci_bus), "VGA");
>> +    qdev_prop_set_int32(dev, "addr", -1);
>> +    qdev_prop_set_bit(dev, "edid", true);
>> +    qdev_init_nofail(dev);
> 
> Hmm.  IMO you should not overwrite the device defaults here.
> 
> edid is off by default only because it is new and I'm conservative.
> I want a release (or two) with it being available for user testing.
> If no issues pop up flip it to default on.

Oh, okay. I already have some unreleased guest code that makes use of this, so my
questions would be: how can EDID be enabled from the command line for in-built VGA
devices, and how do I detect whether EDID support is present from the guest?
Otherwise a guest driver that assumes it is always present and tries to read from
that area of memory will crash.


ATB,

Mark.

Re: [Qemu-devel] [PATCH 2/3] mac_newworld: enable access to EDID data for the VGA device

[Gerd Hoffmann]

On Wed, Dec 12, 2018 at 08:54:37AM +0000, Mark Cave-Ayland wrote:
> On 12/12/2018 08:32, Gerd Hoffmann wrote:
> 
> > On Fri, Dec 07, 2018 at 04:08:05PM +0000, Mark Cave-Ayland wrote:
> >> This is in preparation for some upcoming QEMU NDRV driver changes that pass
> >> display information from the host to the guest.
> > 
> >> -    pci_vga_init(pci_bus);
> >> +    dev = qdev_create(BUS(pci_bus), "VGA");
> >> +    qdev_prop_set_int32(dev, "addr", -1);
> >> +    qdev_prop_set_bit(dev, "edid", true);
> >> +    qdev_init_nofail(dev);
> > 
> > Hmm.  IMO you should not overwrite the device defaults here.
> > 
> > edid is off by default only because it is new and I'm conservative.
> > I want a release (or two) with it being available for user testing.
> > If no issues pop up flip it to default on.
> 
> Oh, okay. I already have some unreleased guest code that makes use of this, so my
> questions would be: how can EDID be enabled from the command line for in-built VGA
> devices,

"-global VGA.edid=on" should do.

> and how do I detect whether EDID support is present from the guest?

Check whenever the header is present.  The edid blob has a fixed 8 byte
header (00 FF FF FF FF FF FF 00).  The linux kernel driver is lazy and
checks the first two bytes only.

> Otherwise a guest driver that assumes it is always present and tries
> to read from that area of memory will crash.

Oh, reading should work no matter what.  You just don't find valid edid
data there if it is turned off.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 2/3] mac_newworld: enable access to EDID data for the VGA device

[Mark Cave-Ayland]

On 12/12/2018 13:38, Gerd Hoffmann wrote:

> On Wed, Dec 12, 2018 at 08:54:37AM +0000, Mark Cave-Ayland wrote:
>> On 12/12/2018 08:32, Gerd Hoffmann wrote:
>>
>>> On Fri, Dec 07, 2018 at 04:08:05PM +0000, Mark Cave-Ayland wrote:
>>>> This is in preparation for some upcoming QEMU NDRV driver changes that pass
>>>> display information from the host to the guest.
>>>
>>>> -    pci_vga_init(pci_bus);
>>>> +    dev = qdev_create(BUS(pci_bus), "VGA");
>>>> +    qdev_prop_set_int32(dev, "addr", -1);
>>>> +    qdev_prop_set_bit(dev, "edid", true);
>>>> +    qdev_init_nofail(dev);
>>>
>>> Hmm.  IMO you should not overwrite the device defaults here.
>>>
>>> edid is off by default only because it is new and I'm conservative.
>>> I want a release (or two) with it being available for user testing.
>>> If no issues pop up flip it to default on.
>>
>> Oh, okay. I already have some unreleased guest code that makes use of this, so my
>> questions would be: how can EDID be enabled from the command line for in-built VGA
>> devices,
> 
> "-global VGA.edid=on" should do.
> 
>> and how do I detect whether EDID support is present from the guest?
> 
> Check whenever the header is present.  The edid blob has a fixed 8 byte
> header (00 FF FF FF FF FF FF 00).  The linux kernel driver is lazy and
> checks the first two bytes only.
> 
>> Otherwise a guest driver that assumes it is always present and tries
>> to read from that area of memory will crash.
> 
> Oh, reading should work no matter what.  You just don't find valid edid
> data there if it is turned off.

Thanks for the info. My current use case for this is passing a set of possible guest
display resolutions from the host to the guest driver, rather than using an internal
hard-coded list. I guess that over time QEMU could become "smarter" about building
the list of supported resolutions depending upon the capabilities of the host?


ATB,

Mark.

Re: [Qemu-devel] [PATCH 2/3] mac_newworld: enable access to EDID data for the VGA device

[Gerd Hoffmann]

  Hi,

> >> Otherwise a guest driver that assumes it is always present and tries
> >> to read from that area of memory will crash.
> > 
> > Oh, reading should work no matter what.  You just don't find valid edid
> > data there if it is turned off.
> 
> Thanks for the info. My current use case for this is passing a set of possible guest
> display resolutions from the host to the guest driver, rather than using an internal
> hard-coded list. I guess that over time QEMU could become "smarter" about building
> the list of supported resolutions depending upon the capabilities of the host?

Yes, that is the plan.  There are xres and yres properties already,
which will show up as preferred resolution in the edid blob.

The generator also supports to set the maximum display resolution, and
the display dpi (needed when we add hidpi support some day).

stdvga doesn't support to change the edid dynamically (i.e. on window
resize) because the hardware doesn't support interrupts, so we can't
easily signal edid updates to the guest.

cheers,
  Gerd

[Qemu-devel] [PATCH 3/3] mac_oldworld: enable access to EDID data for the VGA device

[Mark Cave-Ayland]

This is in preparation for some upcoming QEMU NDRV driver changes that pass
display information from the host to the guest.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/ppc/mac_oldworld.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 9891c325a9..1000570ffd 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -271,7 +271,10 @@ static void ppc_heathrow_init(MachineState *machine)
 
     pci_bus = PCI_HOST_BRIDGE(dev)->bus;
 
-    pci_vga_init(pci_bus);
+    dev = qdev_create(BUS(pci_bus), "VGA");
+    qdev_prop_set_int32(dev, "addr", -1);
+    qdev_prop_set_bit(dev, "edid", true);
+    qdev_init_nofail(dev);
 
     for (i = 0; i < nb_nics; i++) {
         pci_nic_init_nofail(&nd_table[i], pci_bus, "ne2k_pci", NULL);
-- 
2.11.0