Re: Mask bit support's API

["Yang, Sheng" <sheng.yang@intel.com>]

On Wednesday 24 November 2010 09:59:23 Yang, Sheng wrote:
> On Tuesday 23 November 2010 22:06:20 Avi Kivity wrote:
> > On 11/23/2010 03:57 PM, Yang, Sheng wrote:
> > > >  >  Yeah, but won't be included in this patchset.
> > > >  
> > > >  What API changes are needed?  I'd like to see the complete API.
> > > 
> > > I am not sure about it. But I suppose the structure should be the same?
> > > In fact it's pretty hard for me to image what's needed for virtio in
> > > the future, especially there is no such code now. I really prefer to
> > > deal with assigned device and virtio separately, which would make the
> > > work much easier. But seems you won't agree on that.
> > 
> > First, I don't really see why the two cases are different (but I don't
> > do a lot in this space).  Surely between you and Michael, you have all
> > the information?
> > 
> > Second, my worry is a huge number of ABI variants that come from
> > incrementally adding features.  I want to implement bigger chunks of
> > functionality.  So I'd like to see all potential users addressed, at
> > least from the ABI point of view if not the implementation.
> > 
> > > >  The API needs to be compatible with the pending bit, even if we
> > > >  don't implement it now.  I want to reduce the rate of API changes.
> > > 
> > > This can be implemented by this API, just adding a flag for it. And I
> > > would still take this into consideration in the next API purposal.
> > 
> > Shouldn't kvm also service reads from the pending bitmask?
> 
> Of course KVM should service reading from pending bitmask. For assigned
> device, it's kernel who would set the pending bit; but I am not sure for
> virtio. This interface is GET_ENTRY, so reading is fine with it.
> 
> > > >  So instead of
> > > >  
> > > >  - guest reads/writes msix
> > > >  - kvm filters mmio, implements some, passes others to userspace
> > > >  
> > > >  we have
> > > >  
> > > >  - guest reads/writes msix
> > > >  - kvm implements all
> > > >  - some writes generate an additional notification to userspace
> > > 
> > > I suppose we don't need to generate notification to userspace? Because
> > > every read/write is handled by kernel, and userspace just need
> > > interface to kernel to get/set the entry - and well, does userspace
> > > need to do it when kernel can handle all of them? Maybe not...
> > 
> > We could have the kernel handle addr/data writes by setting up an
> > internal interrupt routing.  A disadvantage is that more work is needed
> > if we emulator interrupt remapping in qemu.
> 
> In fact modifying irq routing in the kernel is also the thing I want to
> avoid.
> 
> So, the flow would be:
> 
> kernel get MMIO write, record it in it's own MSI table
> KVM exit to QEmu, by one specific exit reason
> QEmu know it have to sync the MSI table, then reading the entries from
> kernel QEmu found it's an write, so it need to reprogram irq routing table
> using the entries above
> done
> 
> But wait, why should qemu read entries from kernel? By default exit we
> already have the information about what's the entry to modify and what to
> write, so we can use them directly. By this way, we also don't need an
> specific exit reason - just exit to qemu in normal way is fine.
> 
> Then it would be:
> 
> kernel get MMIO write, record it in it's own MSI table
> KVM exit to QEmu, indicate MMIO exit
> QEmu found it's an write, it would update it's own MSI table(may need to
> query mask bit from kernel), and reprogram irq routing table using the
> entries above done
> 
> Then why should kernel kept it's own MSI table? I think the only reason is
> we can speed up reading in that way - but the reading we want to speed up
> is mostly on enabled entry(the first entry), which is already in the IRQ
> routing table...
> 
> And for enabled/disabled entry, you can see it like this: for the entries
> inside routing table, we think it's enabled; otherwise it's disabled. Then
> you don't need to bothered by pci_enable_msix().
> 
> So our strategy for reading accelerating can be:
> 
> If the entry contained in irq routing table, then use it; otherwise let
> qemu deal with it. Because it's the QEmu who owned irq routing table, the
> synchronization is guaranteed. We don't need the MSI table in the kernel
> then.
> 
> And for writing, we just want to cover all of mask bit, but none of others.
> 
> I think the concept here is more acceptable?
> 
> The issue here is MSI table and irq routing table got duplicate information
> on some entries. My initial purposal is to use irq routing table in
> kernel, then we don't need to duplicate information.

Avi?

And BTW, we can take routing table as a kind of *cache*, if the content is in the 
cache, then we can fetch it from the cache, otherwise we need to go back to fetch 
it from memory(userspace). 

--
regards
Yang, Sheng

> 
> 
> --
> regards
> Yang, Sheng