Re: [kvm-ppc-devel] KVM kernel/userspace TLB interface

[Christian E. <ehr...@li...>]

It looks like we have two different needs we can solve with the approach Hollis suggested by implementing #2 (arch specific) and #3 (generic map this) interfaces.

Use cases - for #2 (set/get TLB elements including architecture specific information):
- Dump TLB, comparable to vcpu_dump we will need a tlb_dump for debugging and error reporting.
- Save/Restore/Migrate - even not being a need right now this might become a need for us all end so we will need a get/set function that is able to completely save/restore the guest TLB

Use Cases for #3 (insert a single mapping )
- simplifying shared userspace code by using only one common interface wherever possible that does not need to know which hardware it adresses

---

#2 - "get/set TLB"
I attached a prototype implementation of an architecture dependent mass KVM_GET_TLB & KVM_SET_TLB which get and set the full status the guest & shadow tlb structs assigned to a vcpu.
I intend to eventually have different calls for guest and shadow structure because e.g. we actually will never want to set shadow tlb from userspace - this prototype is still in development and any comments are welcome (I'll resend it once it is changed to different getter/setter for guest and shadow tlb).
@Zhang Wei - the interface I have in mind for this arch specific implementation is an array of tlbe[MAX_TLB_SIZE] get-/setting all elements of either guest or shadow tlb at once. Every architecture can define it's own tlbe structure in the kernel headers and can implement special needs in their backend for these ioctls (KVM_GET_GUEST_TLB, KVM_GET_SHADOW_TLB, KVM_SET_GUEST_TLB, but no KVM_SET_SHADOW_TLB).

---

#3 - "map this"
For #3 we need some discussion how the data structure of a shared interface might look like. For ppc440 it would be the { virtual addr, real addr, size, flags } as you see the tlbe definition that is moved in my patch. I wanted to ask what more/less needs e500 has for a "map this" interface to insert a single tlb-entry. We need also to clarify what flags we all need to define a shared set of bits (if possible to share that flags) in the flags variable we can use that every architecture can map to it's own meaning in the backend to this ioctl.
When I know the needs of e500 for the "map this" interface (and any other needs brought up that might affect future hardware we want to be compatible with) I intend to implement a prototype of this interface for ppc440 and then iterate it until we are happy with it for ppc440, e500 and whatever hw we have in mind for the future.


Zhang Wei wrote:
> Hi, Hollis,
> 
>> Hi Zhang, the administrative tools have a need to access the guest's TLB
>> state, for example to create the mapping for the initial code to execute
>> in. This requires a kernel interface to be exported to userspace, but as
>> you noted, the TLBs are very different between 440 and e500.
>>
>> Here are a couple ideas:
[...]
>>      #2* Have a per-core interface, e.g. KVM_440_SET_TLB_ENTRY and
>>         KVM_e500_SET_TLB_ENTRY. These would communicate via completely
>>         different structures. For example, the 440 structure 
>> would look
>>         something like { index, TID, word 0, word 1, word 2 }. We'd
>>         probably want to sanity check each ioctl to make sure it matches
>>         the vcpu type. Not only that, but we would need core-specific
>>         code in userspace to actually make the call.
>>      #3* Try to abstract it so that we have a higher-level "map this"
>>         interface. This could look something like { virtual addr, real
>>         addr, size, flags }. It's up to core-specific code to select in
>>         which TLB to create the mapping, which entry to clobber,
>>         interpret the meaning of "flags", etc. I guess the big downside
>>         here is that if we want to dump guest state for debugging
>>         purposes we'd still need the core-specific interface.
>>
>> We could also try supporting both #2 and #3, so that 
>> "generic" code like
>> qemu could use the abstracted (and shared) "map this" call, 
>> while tools
>> with more specific needs could still get at the underlying details.
>>
> 
> Agree with you!
> In my porting implementation, I used #3. I think #3 should be more
> flexible.
> Maybe we'll add the other TLB support to it in the future. We should
> define a good data structure about TLB content, which will include 440
> and E500 TLB meaning.
> 
> And If we take out TLB operations, rest codes are more generic.
> 
> Cheers!
> Wei.

And Avi Kivity wrote:
> Hollis Blanchard wrote:   
> [...]
> An unabstracted interface is needed for save/restore/migrate, which may 
> or may not be important for your target.  I do recommend having getters 
> and setters for all architectural state.
>

-- 

Grüsse / regards, 
Christian Ehrhardt
IBM Linux Technology Center, Open Virtualization