Christian Ehrhardt wrote:

This mail now has the reworked implementation of the patch for ppc440 in the #2 style attached where the guest tlb can be get/set completely and the shadow tlb can be read from userspace (now separate ioctls for guest/shadow).
Actually this should work for most if not all use cases we have. Especially our current use case of setting up a proper initial state is very arch dependent anyway and if we really can save userspace code for shared usage depends on the fact if we will be able to create a shared bit format for flags (I start to put a common "map this" call down on my prio list).

I just downloaded the "PowerPC e500 Core Family Reference Manual" and will read a bit to see where the differences are in relation to this interface, but atm everything works for me because the #2 style interface in this patch is sufficient for the only use case we have atm.
@Zhang Wei: I would really like to see your code for the solution in the "map this" style you have mentioned that you have written for e500.

> 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