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

[Leif Lindholm]

On Thu, Aug 06, 2015 at 02:28:03PM +0200, Andrew Jones wrote:
> On Thu, Aug 06, 2015 at 12:24:27PM +0100, Leif Lindholm wrote:
> > The _ADR entry in SPCR is optional and redundant. The same information
> > is already provided in _CRS (which is mandatory).
> > 
> > Signed-off-by: Leif Lindholm <address@hidden>
> > ---
> > 
> > So, this _ADR entry is only consumed by a set of not-widely-circulated
> > patches for the Linux kernel. And while the ARM Server Base Boot
> > Requirements specification mandates SPCR, it does not mandate this _ADR
> > entry.
> > 
> > In the interest of not propagating non-standard extensions, I would be
> > really happy if we could consider dropping this from 2.4.
> > I do realize that this is a completely unreasonable request this late
> > in the release process, but I only spotted this yesterday, and it is a
> > very isolated change with very quantifiable effects.
> > 
> > The patch at 
> > https://git.linaro.org/leg/acpi/leg-kernel.git/commitdiff/46eeec7b7332bdd104941703696d3812afd934c8
> > converts the non-upstream kernel SPCR handling code to use the _CRS
> > information instead.
> 
> Grr... So when I saw how the kernel (the original non-upstream patch)
> was using ADR, I presumed that that was a documented behavior. I guess
> I should have confirmed that...

Yeah, I found it the other way, by SCPR not working on a platform that
clearly had it defined.

> While the kernel change makes sense, I'm not sure we want this QEMU
> patch, as there *are* kernels already using ADR.

Well ... yes, there are kernels with out-of-tree patches that have
never worked with released versions of QEMU.

Could a variant of my kernel patch, but with an "_ADR" fallback be
added to those kernels? I could whip that up in a couple of minutes.

> In the least I wouldn't want to get burned twice, so I'd prefer to
> see the SPCR code actually get into Linux first this time. That
> would also allow us to point at something when we start breaking
> guests.

So, if that's the way it has to be, that's the way it has to be.
I'd just prefer not having different pieces of firmware validating
different software behaviours for the same thing.

If we can't just rip it out, could we at least stick a warning comment
in a blink tag next to the _ADR addition?

> Actually, why was ADR used the first time? It would be good to know
> the story there in case there as a valid reason.

The phrase "it was just a quick hack" was whispered in my ear...
But since it was never reviewed in public, the reasons were never
discussed.

/
    Leif