Re: [Qemu-ppc] [PATCH] spapr: Add ibm, processor-storage-keys property to CPU DT node

[Ram Pai]

On Thu, Aug 24, 2017 at 12:54:48PM +1000, Paul Mackerras wrote:
> On Mon, Aug 21, 2017 at 05:00:36PM -0300, Thiago Jung Bauermann wrote:
> > LoPAPR says:
> > 
> >     “ibm,processor-storage-keys”
> > 
> >     property name indicating the number of virtual storage keys supported
> >     by the processor described by this node.
> > 
> >     prop-encoded-array: Consists of two cells encoded as with encode-int.
> >     The first cell represents the number of virtual storage keys supported
> >     for data accesses while the second cell represents the number of
> >     virtual storage keys supported for instruction accesses. The cell value
> >     of zero indicates that no storage keys are supported for the access
> >     type.
> > 
> > pHyp provides the property above but there's a bug in P8 firmware where the
> > second cell is zero even though POWER8 supports instruction access keys.
> > This bug will be fixed for P9.
> > 
> > Tested with KVM on POWER8 Firenze machine and with TCG on x86_64 machine.
> > 
> > Signed-off-by: Thiago Jung Bauermann <address@hidden>
> > ---
> > 
> > The sysfs files are provided by this patch for Linux:
> 
> Those sysfs files relate to the kernel's support for userspace
> processes using storage keys.  That is quite distinct from KVM's
> support for guests using storage keys, so I think that using those
> sysfs files to indicate what the guest can do is wrong.
> 
> In fact KVM allows guests to specify storage keys in the HPTE values
> that they set, except that there is a bug (for which Ram Pai has
> posted a patch) that means that KVM loses the top two bits of the key
> number.
> 
> What I would suggest is that we use the 'pad' field in the struct
> kvm_ppc_smmu_info to report the number of keys supported by KVM for
> guest use.  That will be 0 in all current kernels, indicating that
> keys are not supported, which is reasonable because of the bug.  I
> will make sure the patch fixing the bug goes in first.

with the current kernels, even with the bug, KVM can still support 8
keys. Should we say 8 instead of 0?  It will help enable keys on KVM
earlier and give a jump start to its adaption by applications.

RP