On Thu, Aug 24, 2017 at 02:15:01PM +1000, Paul Mackerras wrote: > On Thu, Aug 24, 2017 at 02:02:43PM +1000, David Gibson wrote: > > 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 <bauer...@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. > > > > Ah! Glad someone pointed that out. > > > > > 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. > > > > Is there a particular reason to put it there rather than a new KVM > > capability? > > Just that storage keys are an aspect of the "server" (i.e. HPT) MMU, > so it seemed to make sense to put it together with the other > information about the HPT MMU (segment sizes, page sizes, etc.).
Ok, works for me.
>
> > > 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.
> >
> > Note that the same migration concerns still apply, so we'll still want
> > machine properties to control this in qemu, which are validated
> > against what the host can do. Since current kernels are buggy, I
> > suggest we have these properties default to 0 - i.e. you need to
> > explicitly request storage keys on the command line if you want your
> > guest to use them. Once fixed kernels are rolled out we can look at
> > changing that in a future machine type.
> >
> > > 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?
> >
> > I'd prefer the separate numbers for I and D. It's more flexible and
> > no harder to implement AFAICT.
>
> OK.
>
>
> Paul.
>
--
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
signature.asc
Description: PGP signature
