Re: [Qemu-devel] [PATCH 00/10] Clean up page size handling for ppc 64-bit hash MMUs with TCG

[Alexander Graf]

> Am 26.01.2016 um 06:41 schrieb David Gibson <address@hidden>:
> 
>> On Mon, Jan 25, 2016 at 09:36:40PM +0100, Alexander Graf wrote:
>> 
>> 
>>> On 01/25/2016 12:10 PM, David Gibson wrote:
>>>> On Mon, Jan 25, 2016 at 04:15:42PM +1100, David Gibson wrote:
>>>> Encoding of page sizes on 64-bit hash MMUs for Power is rather arcane,
>>>> involving control bits in both the SLB and HPTE.  At present we
>>>> support a few of the options, but far fewer than real hardware.
>>>> 
>>>> We're able to get away with that in practice, because guests use a
>>>> device tree property to determine which page sizes are available and
>>>> we are setting that to match.  However, the fact that the actual code
>>>> doesn't necessarily what we put into the table of available page sizes
>>>> is another ugliness.
>>>> 
>>>> This series makes a number of cleanups to the page size handling.  The
>>>> upshot is that afterwards the softmmu code operates off the same page
>>>> size encoding table that is advertised to the guests, ensuring that
>>>> they will be in sync.
>>>> 
>>>> Finally, we extend the table of allowed sizes for POWER7 and POWER8 to
>>>> include the options allowed in hardware (including MPSS).  We can fix
>>>> other hash MMU based CPUs in future if anyone cares enough.
>>>> 
>>>> Please review, and I'll fold into ppc-for-2.6 for my next pull.
>>> Bother, somehow missed a serious bug in here that's causing
>>> oops-on-boot.  Sorry, still tracking it down.
>> 
>> I still have no idea where your bug is (bisect probably should get you there
>> pretty quick),
> 
> Alas, no, because the bug only triggers once all the page sizes are
> added for POWER8 in the last patch.
> 
> Luckily I found it anyway (see earlier reply).
> 
>> but the overall concept sounds very reasonable to me. Please
>> benchmark performance before and after in the next cover letter also
>> :)
> 
> Hrm.. as always the question is what benchmark?

For this "time" on a simple kernel boot+automatic shutdown sequence should be 
enough.

Alex

> 
>> 
>> Reviewed-by: Alexander Graf <address@hidden>
>> 
>> Alex
> 
> -- 
> 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