Re: [Qemu-devel] [RFC] QOMification of AXI streams

[Benjamin Herrenschmidt]

On Mon, 2012-06-11 at 20:33 -0500, Anthony Liguori wrote:
> On 06/11/2012 06:46 PM, Benjamin Herrenschmidt wrote:
> > On Mon, 2012-06-11 at 17:29 -0500, Anthony Liguori wrote:
> >
> >> When we add a subregion, it always removes the offset from the address 
> >> when it
> >> dispatches.  This more often than not works out well but for what you're
> >> describing above, it sounds like you'd really want to get an adjusted size 
> >> (that
> >> could be transformed).
> >>
> >> Today we generate a linear dispatch table.  This prevents us from applying
> >> device-level transforms.
> >
> > subregions being relative to the parent would probably work for me.
> >
> > Typically I have something like:
> >
> >    0x1234_0000_0000 .. 0x1234_3fff_ffff : window to PCI memory
> >
> > Which generates PCI cycles into the range:
> >
> >    0x0000_c000_0000 .. 0x0000_ffff_ffff
> >
> > Which is a combination of bit masking&  offset, ie, the HW algorithm is
> > to forward some bits (0x3fff_ffff in this case) and replace the other
> > ones (with 0xc000_0000) in this case.
> >
> > Is that properly represented in the current scheme by a subregion ?
> 
> Does this mean that both 0x1234..0000_0000is a valid address to issue PCI 
> requests along with 0x0000_c000_000?
> 
> If they both are valid, that sounds pretty much like what aliases were made 
> for.

No. 0x1234_0000_0000 is what the CPU accesses (the window into the CPU
space) and an access to that address will generate a PCI access with
address 0xc000_0000 on the PCI bus.

> >> So if you stick with the notion of subregions, you would still have a 
> >> single
> >> MemoryRegion at the PCI bus layer that has all of it's children as sub 
> >> regions.
> >>    Presumably that "scan for all siblings" is a binary search which 
> >> shouldn't
> >> really be that expensive considering that we're likely to have a shallow 
> >> depth
> >> in the memory hierarchy.
> >
> > Possibly but on every DMA access ...
> 
> We can always as a TLB :-)

Yeah... at least up to the first guy who tries to translate... unless we
start caching translations too :-)

> >>> Additionally to handle iommu's etc... you need the option for a given
> >>> memory region to have functions to perform the transformation in the
> >>> upstream direction.
> >>
> >> I think that transformation function lives in the bus layer MemoryRegion.  
> >> It's
> >> a bit tricky though because you need some sort of notion of "who is 
> >> asking".  So
> >> you need:
> >>
> >> dma_memory_write(MemoryRegion *parent, DeviceState *caller,
> >>                    const void *data, size_t size);
> >
> > Why do you need the parent argument ? Can't it be implied from the
> > DeviceState ? Or do you envision devices sitting on multiple busses ? Or
> > it's just a matter that each device "class" will store that separately ?
> 
> Yeah, you may have multiple MemoryRegions you want to write to.  I don't 
> think 
> we should assume all devices have exactly one address space it writes to.

Ok.

> > Also you want the parent to be the direct parent P2P no ? Not the host
> > bridge ? Ie. to be completely correct, you want to look at every sibling
> > device under a given bridge, then go up if there is no match, etc...
> > until you reach the PHB at which point you hit the iommu and eventually
> > the system fabric.
> 
> Yes.
> 
> >> Not quite...  subtractive decoding only happens for very specific devices 
> >> IIUC.
> >>    For instance, an PCI-ISA bridge.  Normally, it's positive decoding and a
> >> bridge has to describe the full region of MMIO/PIO that it handles.
> >
> > That's for downstream. Upstream is essentially substractive as far as I
> > can tell. IE. If no sibling device decodes the cycle, then it goes up
> > no ?
> 
> Why would that be necessary?  Are upstream requests p2p?  I would have 
> assumed 
> that they went to the host bus and then were dispatched by the host bus (but 
> I'm 
> just guess).

On classic PCI there's no such thing:

  Host
   |
  P2P
   |-----|-----|
 dev0  dev1  dev2

In a setup like the above, if dev0 does a write to address A, from the
perspective of the bus segment where dev0,1 and 2 sit, this is just an
initiator requesting arbitration, and when granted it, doing a write to
address A. There is no concept of "upstream". The P2P bridge downstream
leg, for all intent and purpose is just another device on that bus
segment.

So it depends who decodes A and if neither dev1 nor dev2 does, then the
P2P gets it and passes it up the food chain (it doesn't have a
configurable "range" of upstream addresses to decode). 

> >> So it's only necessary to transverse down the tree again for the very 
> >> special
> >> case of PCI-ISA bridges.  Normally you can tell just by looking at 
> >> siblings.
> >>
> >>> That will be a significant overhead for your DMA ops I believe, though
> >>> doable.
> >>
> >> Worst case scenario, 256 devices with what, a 3 level deep hierarchy?  
> >> we're
> >> still talking about 24 simple address compares.  That shouldn't be so bad.
> >
> > Per DMA access...
> >
> >>> Then we'd have to add map/unmap to MemoryRegion as well, with the
> >>> understanding that they may not be supported at every level...
> >>
> >> map/unmap can always fall back to bounce buffers.
> >
> > Right.
> >
> >>> So yeah, it sounds doable and it would handle what DMAContext doesn't
> >>> handle which is access to peer devices without going all the way back to
> >>> the "top level", but it's complex and ... I need something in qemu
> >>> 1.2 :-)
> >>
> >> I think we need a longer term vision here.  We can find incremental 
> >> solutions
> >> for the short term but I'm pretty nervous about having two parallel APIs 
> >> only to
> >> discover that we need to converge in 2 years.
> >
> > Can we agree that what we need to avoid having to change every second
> > day is the driver API ?
> >
> > In which case, what about I come up with some pci_* DMA APIs such as the
> > ones you suggested above and fixup a handful of devices we care about to
> > them.
> 
> Yes!
> 
> If I recall, that was what I suggested originally and you talked me out of it 
> :-)
> 
> I think that's the best future proof design.

Yeah well, I don't remember exactly and we all make mistakes but bus
specific helpers are a good idea, I don't remember talking you out of
that specifically :-)

I'll respin the patches along those lines so we can easily move toward
something fancier without having to impact all devices.

Cheers,
Ben.

> Regards,
> 
> Anthony Liguori
> 
> >
> > Under the hood, we can make it use the DMAContext for now and have that
> > working for us, but we can easily "merge" DMAContext and MemoryRegion
> > later since it's not directly exposed to devices.
> >
> > IE. The devices would only use:
> >
> >    pci_device_read/write/map/unmap(PCIDevice,...)
> >
> > And DMAContext remains burried in the implementation.
> >
> > Would that be ok with you ?
> >
> > Cheers,
> > Ben.
> >
> >> Regards,
> >>
> >> Anthony Liguori
> >>
> >>
> >>> In addition there's the memory barrier business so we probably want to
> >>> keep the idea of having DMA specific accessors ...
> >>>
> >>> Could we keep the DMAContext for now and just rename it to MemoryRegion
> >>> (keeping the accessors) when we go for a more in depth transformation ?
> >>>
> >>> Cheers,
> >>> Ben.
> >>>
> >>>
> >>>
> >
> >