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

[Paul Mackerras <paulus@ozlabs.org>]

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 <bauerman@linux.vnet.ibm.com>
> ---
> 
> 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.

We could either have two u16 fields for the number of keys for data
and instruction, or we could have a u32 field for the number of keys
and a separate bit in the flags field to indicate that instruction
keys are supported.  Which would be preferable?

Paul.