Implementing IOMMU support for SDHCI

Implementing IOMMU support for SDHCI

[Sai Pavan Boddu]

Hi Poalo/Peter,

We are trying to add SMMU support for dma's used in SD controller on ZynqMP boards. In order to do that we need a clean way to assign MemTxAttrs to each device(i.e all devices which connect to smmu) via "object_add_link_property/ object_set_link_property "  functions.

But in order to do that, first we may need to convert MemTxAttrs as QOM object. I just need some feedback, if its advisable to do so ?

Thanks & Regards,
Sai Pavan

Re: Implementing IOMMU support for SDHCI

[Peter Maydell]

On Wed, 26 Feb 2020 at 12:38, Sai Pavan Boddu <saipava@xilinx.com> wrote:
>
> Hi Poalo/Peter,
>
> We are trying to add SMMU support for dma's used in SD controller on ZynqMP boards. In order to do that we need a clean way to assign MemTxAttrs to each device(i.e all devices which connect to smmu) via "object_add_link_property/ object_set_link_property "  functions.
>
> But in order to do that, first we may need to convert MemTxAttrs as QOM object. I just need some feedback, if its advisable to do so ?

We create and throw around MemTxAttrs structs a lot, I don't think they're
suitable to be QOM objects. I don't think you need it, anyway -- your
problem is rather that the iommu doesn't get an opportunity to sit in
the middle of transactions the way you'd like it to, and making
MemTxAttrs a QOM object doesn't help address that.

thanks
-- PMM

RE: Implementing IOMMU support for SDHCI

[Sai Pavan Boddu]

Hi Peter,

> -----Original Message-----
> From: Peter Maydell <peter.maydell@linaro.org>
> Sent: Wednesday, February 26, 2020 9:59 PM
> To: Sai Pavan Boddu <saipava@xilinx.com>
> Cc: pbonzini@redhat.com; QEMU Developers <qemu-devel@nongnu.org>;
> Edgar Iglesias <edgari@xilinx.com>
> Subject: Re: Implementing IOMMU support for SDHCI
> 
> On Wed, 26 Feb 2020 at 12:38, Sai Pavan Boddu <saipava@xilinx.com>
> wrote:
> >
> > Hi Poalo/Peter,
> >
> > We are trying to add SMMU support for dma's used in SD controller on
> ZynqMP boards. In order to do that we need a clean way to assign
> MemTxAttrs to each device(i.e all devices which connect to smmu) via
> "object_add_link_property/ object_set_link_property "  functions.
> >
> > But in order to do that, first we may need to convert MemTxAttrs as QOM
> object. I just need some feedback, if its advisable to do so ?
> 
> We create and throw around MemTxAttrs structs a lot, I don't think they're
> suitable to be QOM objects. I don't think you need it, anyway -- your problem
> is rather that the iommu doesn't get an opportunity to sit in the middle of
> transactions the way you'd like it to, and making MemTxAttrs a QOM object
> doesn't help address that.
[Sai Pavan Boddu] Our idea is to create dma helper function which can also pass the MemTxAttr i.e like 
dma_memory_rw_attr (AddressSpace *as, dma_addr_t addr,
                                     void *buf, dma_addr_t len,
                                     DMADirection dir,
                                     MemTxAttrs attr)
which needs to be used in sdhci,  with "AddressSpace *as" pointing to smmu.

I see we don’t need QOM variant of MemTxAttr for this functionality, but thought it would be good to use  " object_add_link_property/ object_set_link_property " to set the values from machine file's. I would drop this idea, and look for other options to set the attributes.

Regards,
Sai Pavan

> 
> thanks
> -- PMM

Re: Implementing IOMMU support for SDHCI

[Peter Maydell]

On Fri, 28 Feb 2020 at 10:08, Sai Pavan Boddu <saipava@xilinx.com> wrote:
> [Sai Pavan Boddu] Our idea is to create dma helper function which can also pass the MemTxAttr i.e like
> dma_memory_rw_attr (AddressSpace *as, dma_addr_t addr,
>                                      void *buf, dma_addr_t len,
>                                      DMADirection dir,
>                                      MemTxAttrs attr)
> which needs to be used in sdhci,  with "AddressSpace *as" pointing to smmu.

Yes, you want to be able to pass MemTxAttrs from devices using
the dma APIs. (Or you could just call dma_barrier() yourself
before directly using an address_space_* function.) Plumbing
that through is straightforward.

As an aside, we are enormously inconsistent about whether
devices use dma_memory_* functions or just directly
do address_space_* calls. I think the intention is that
the dma_memory_* include necessary memory barriers, but it's
not clear to me at least when the barriers are necessary or
why we have this family of functions that some devices use
and most just don't.

> I see we don’t need QOM variant of MemTxAttr for this functionality,
>but thought it would be good to use  " object_add_link_property/
>object_set_link_property " to set the values from machine file's.
>I would drop this idea, and look for other options to set the attributes.

The attributes of a transaction are set by the thing generating
the transactions (either the CPU, or the DMA device, whatever it
is). Typically they're either fixed, or possibly configurable
at a hardware level, or sometimes configurable by software.
I don't think any of those scenarios are ones where you'd want
the board model to be passing in a MemTxAttr at device creation
time.

thanks
-- PMM

RE: Implementing IOMMU support for SDHCI

[Sai Pavan Boddu]

Hi Peter,

> -----Original Message-----
> From: Peter Maydell <peter.maydell@linaro.org>
> Sent: Friday, February 28, 2020 3:56 PM
> To: Sai Pavan Boddu <saipava@xilinx.com>
> Cc: pbonzini@redhat.com; QEMU Developers <qemu-devel@nongnu.org>;
> Edgar Iglesias <edgari@xilinx.com>
> Subject: Re: Implementing IOMMU support for SDHCI
> 
> On Fri, 28 Feb 2020 at 10:08, Sai Pavan Boddu <saipava@xilinx.com> wrote:
> > [Sai Pavan Boddu] Our idea is to create dma helper function which can
> > also pass the MemTxAttr i.e like dma_memory_rw_attr (AddressSpace *as,
> dma_addr_t addr,
> >                                      void *buf, dma_addr_t len,
> >                                      DMADirection dir,
> >                                      MemTxAttrs attr) which needs to
> > be used in sdhci,  with "AddressSpace *as" pointing to smmu.
> 
> Yes, you want to be able to pass MemTxAttrs from devices using the dma
> APIs. (Or you could just call dma_barrier() yourself before directly using an
> address_space_* function.) Plumbing that through is straightforward.
> 
> As an aside, we are enormously inconsistent about whether devices use
> dma_memory_* functions or just directly do address_space_* calls. I think
> the intention is that the dma_memory_* include necessary memory barriers,
> but it's not clear to me at least when the barriers are necessary or why we
> have this family of functions that some devices use and most just don't.
> 
> > I see we don’t need QOM variant of MemTxAttr for this functionality,
> >but thought it would be good to use  " object_add_link_property/
> >object_set_link_property " to set the values from machine file's.
> >I would drop this idea, and look for other options to set the attributes.
> 
> The attributes of a transaction are set by the thing generating the
> transactions (either the CPU, or the DMA device, whatever it is). Typically
> they're either fixed, or possibly configurable at a hardware level, or
> sometimes configurable by software.
> I don't think any of those scenarios are ones where you'd want the board
> model to be passing in a MemTxAttr at device creation time.
[Sai Pavan Boddu] Yeah ok, this makes sense. Only things which me might need to configure are master id's and trust zone settings if possible.
May be we can set them at soc emulation level i.e "hw/arm/xlnx-zynqmp.c"

Regards,
Sai Pavan
> 
> thanks
> -- PMM

Re: Implementing IOMMU support for SDHCI

[Peter Maydell]

On Tue, 3 Mar 2020 at 06:05, Sai Pavan Boddu <saipava@xilinx.com> wrote:
> > From: Peter Maydell <peter.maydell@linaro.org>
> > I don't think any of those scenarios are ones where you'd want the board
> > model to be passing in a MemTxAttr at device creation time.
> [Sai Pavan Boddu] Yeah ok, this makes sense. Only things which me might need to configure are master id's and trust zone settings if possible.
> May be we can set them at soc emulation level i.e "hw/arm/xlnx-zynqmp.c"

For that kind of thing, a good guide is to look at what the
hardware does. If the device inside the SoC just has
a hardcoded master ID, hardcode it in the QEMU device
model. If it has some signal lines that the SoC has to tie
off to 1 or 0 to configure it, implement that as a QEMU
device property (usually with the same name and basic
semantics as the config signal lines).

thanks
-- PMM