Re: [Qemu-devel] [PATCH] hw/arm/virt-acpi-build: drop _ADR entry from SPCR

[Andrew Jones]

On Thu, Aug 20, 2015 at 12:21:31PM +0100, Leif Lindholm wrote:
> On Thu, Aug 20, 2015 at 07:09:57PM +0800, Shannon Zhao wrote:
> > >>>Could somebody who understands ACPI and the ramifications
> > >>>here let me know if I should apply this patch, please?
> > >>>(since we're now post-2.4)
> > >>
> > >>I presume my opinion is clear, but I'm cc:ing some of the Linaro ACPI
> > >>team.
> > >>
> > >>Graeme, Al - the patch in question is:
> > >>https://www.mail-archive.com/qemu-devel%40nongnu.org/msg314356.html
> > >>
> > >Using _ADR for a non enumerable bus is undefined behaviour in the ACPI
> > >specification.
> > >
> > >How it is used in Redhats SPCR patch is IMO wrong becuase there is no
> > >guarantee that _ADR will be defined for any MMIO device in DSDT.
> > >
> > >I believe QEMU should not follow this just to make a non upstreamed
> > >Redhat patch work.

Well, it's a shame that the kernel patch that used ADR was committed to
Red Hat's and Linaro's trees before it had been thought through
completely, but it was. 

> > >
> > Yeah, but when will the right kernel patch be upstreamed? Do you
> > have a plan for upstreaming it? Or it's on the list already?
> 
> It's on my way too long to-do list, but I'll need to send it out in
> whatever state as an RFC this week anyway.
> 
> > As said before, we can apply this patch after the kernel patch upstreamed.
> 
> Meanwhile, it would be very bad if this becomes a de-facto standard,
> using QEMU as a vector to (needlessly) change specifications through
> the back door.

If I understand correctly, then the concern is that vendors, ones which
use QEMU code as their specification, will start building ACPI tables
with ADR unnecessarily populated in the console uart's device table.
Actually, some vendors must have already been doing that, otherwise the
out-of-tree patches in RH's and Linaro's trees wouldn't have worked on
bare-metal. So, what is the problem with them doing it? Just wrong
because it's pointless?

If I'm right about the concerns, then I don't see why we should rush
this QEMU change. Also, it would be much easier to apologize to the guest
kernels that the change will break, if we can point at an upstream patch
that they need to backport. I.e. I still vote that we wait for the kernel
patch to get upstream first.

I'll also reiterate the obvious fact that the kernel can switch to CRS
whenever it likes. That'll work just fine with or without QEMU taking
this change.

Thanks,
drew