[RFC] KVM: PPC: Book3S HV: Reserve POWER8 space in get/set_one_reg

Alex,

This reserves space in get/set_one_reg ioctl for the extra guest state
needed for POWER8.  It doesn't implement these at all, it just reserves
them so that the ABI is defined now.  

A few things to note here:

- POWER8 has 6 PMCs and an additional 2 SPMCs for the supervisor.  Here
  I'm storing these 2 SPMCs in PMC7/8.

- This add *a lot* state for transactional memory.  TM suspend mode,
  this is unavoidable, you can't simply roll back all transactions and
  store only the checkpointed state.  I've added this all to
  get/set_one_reg (including GPRs) rather than creating a new ioctl
  which returns a struct kvm_regs like KVM_GET_REGS does.  This means we
  if we need to extract the TM state, we are going to need a bucket load
  of IOCTLs.  Hopefully most of the time this will not be needed as we
  can look at the MSR to see if TM is active and only grab them when
  needed.

- The TM state is offset bu 0x1000.  Other than being bigger than the
  SPR space, it's fairly arbitrarily chose. 

- For TM, I've done away with VMX and FP and created a single 64x128 bit
  VSX register space.

Alex: I'll add the documentation Documentation/virtual/kvm/api.txt if
you're happy with all this.

Signed-off-by: Michael Neuling <mikey@neuling.org>


--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On 30.08.2013, at 08:09, Michael Neuling wrote:

> Alex,
> 
> This reserves space in get/set_one_reg ioctl for the extra guest state
> needed for POWER8.  It doesn't implement these at all, it just reserves
> them so that the ABI is defined now.  
> 
> A few things to note here:
> 
> - POWER8 has 6 PMCs and an additional 2 SPMCs for the supervisor.  Here
>  I'm storing these 2 SPMCs in PMC7/8.

Is this safe to do? Are we guaranteed that POWER9 or POWER10 doesn't introduce a real PMC7?

> - This add *a lot* state for transactional memory.  TM suspend mode,
>  this is unavoidable, you can't simply roll back all transactions and
>  store only the checkpointed state.  I've added this all to
>  get/set_one_reg (including GPRs) rather than creating a new ioctl
>  which returns a struct kvm_regs like KVM_GET_REGS does.  This means we
>  if we need to extract the TM state, we are going to need a bucket load
>  of IOCTLs.  Hopefully most of the time this will not be needed as we
>  can look at the MSR to see if TM is active and only grab them when
>  needed.

If we find this to be a performance issue, we can always add a new ioctl that allows multiple ONE_REG accesses at a time. The only reason we don't have that yet is that bulk one_reg access hasn't happened in any performance critical path so far.

> 
> - The TM state is offset bu 0x1000.  Other than being bigger than the
>  SPR space, it's fairly arbitrarily chose. 
> 
> - For TM, I've done away with VMX and FP and created a single 64x128 bit
>  VSX register space.
> 
> Alex: I'll add the documentation Documentation/virtual/kvm/api.txt if
> you're happy with all this.

Looks perfectly reasonable to me :).


Alex

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Fri, 2013-08-30 at 16:01 +0200, Alexander Graf wrote:
> > 
> > - The TM state is offset bu 0x1000.  Other than being bigger than
> the
> >  SPR space, it's fairly arbitrarily chose. 

Make it higher, just in case....

Ben.


--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Sat, Aug 31, 2013 at 8:17 AM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Fri, 2013-08-30 at 16:01 +0200, Alexander Graf wrote:
>> >
>> > - The TM state is offset bu 0x1000.  Other than being bigger than
>> the
>> >  SPR space, it's fairly arbitrarily chose.
>
> Make it higher, just in case....

Ok but how high?  KVM_REG_SiZE is set by generic code and it's way up
at 0x0030000000000000ULL

Mikey
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Sat, Aug 31, 2013 at 12:01 AM, Alexander Graf <agraf@suse.de> wrote:
>
> On 30.08.2013, at 08:09, Michael Neuling wrote:
>
>> Alex,
>>
>> This reserves space in get/set_one_reg ioctl for the extra guest state
>> needed for POWER8.  It doesn't implement these at all, it just reserves
>> them so that the ABI is defined now.
>>
>> A few things to note here:
>>
>> - POWER8 has 6 PMCs and an additional 2 SPMCs for the supervisor.  Here
>>  I'm storing these 2 SPMCs in PMC7/8.
>
> Is this safe to do? Are we guaranteed that POWER9 or POWER10 doesn't introduce a real PMC7?

Good point.  I'll change it.

>
>> - This add *a lot* state for transactional memory.  TM suspend mode,
>>  this is unavoidable, you can't simply roll back all transactions and
>>  store only the checkpointed state.  I've added this all to
>>  get/set_one_reg (including GPRs) rather than creating a new ioctl
>>  which returns a struct kvm_regs like KVM_GET_REGS does.  This means we
>>  if we need to extract the TM state, we are going to need a bucket load
>>  of IOCTLs.  Hopefully most of the time this will not be needed as we
>>  can look at the MSR to see if TM is active and only grab them when
>>  needed.
>
> If we find this to be a performance issue, we can always add a new ioctl that allows multiple ONE_REG accesses at a time. The only reason we don't have that yet is that bulk one_reg access hasn't happened in any performance critical path so far.

Ok.

>
>>
>> - The TM state is offset bu 0x1000.  Other than being bigger than the
>>  SPR space, it's fairly arbitrarily chose.
>>
>> - For TM, I've done away with VMX and FP and created a single 64x128 bit
>>  VSX register space.
>>
>> Alex: I'll add the documentation Documentation/virtual/kvm/api.txt if
>> you're happy with all this.
>
> Looks perfectly reasonable to me :).

Thanks.  I'll repost.

Mikey
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html