[illumos-Developer] Webrev #534: DMA KVA cache attributes incorrect

[Garrett D'Amore]

I just walked through the code on this, and I'm pleased to say that your
changes look very much correct for x86... see i_ddi_mem_alloc() on the
x86 platform.

Sadly, on SPARC it isn't right.  On SPARC platforms it is impossible to
allocate uncached memory, because the SPARC platform guarantees cache
consistency.

I *think* this will be harmless because the flags don't appear to be
used on SPARC, although I'm not terribly happy about that.  So I'd at a
minimum add a comment above indicating that the flags are not used on
SPARC.

Note that these IOMEM flags *are* used in some other functions, and if
passed to certain functions on SPARC will cause a message to be logged
to the console.  (I.e. if they are passed in via ddi_dma_mem_alloc() or
via ddi_umem_alloc().)  See i_ddi_check_cache_attr() for more detail.

	- Garrett

On Tue, 2011-05-10 at 19:05 +0400, Michael Tsymbalyuk wrote:
> For test purposes I added a code snipped to the vmxnet3.c
> driver (for my local testing purposes, of course) - since
> we need a real device for dealing with DMA memory allocation. 
> Inside device's attach() logic it allocates 3 DMA areas with different
> attributes:
> 
>    "DDI_STRICTIRDER_ACC",
>    "DDI_MERGING_OK_ACC",
>    "DDI_UNORDERED_OK_ACC"
> 
>  and prints out their addresses.
> 
> Next, I run mdb -k and walk through these addresses using ::vatopfn
> and ::pte commands.
> 
> It shows that areas have the following attributes:
> 
>     DDI_STRICTIRDER_ACC : uncached
>     DDI_MERGING_OK_ACC  : wrcombine
>     DDI_UNORDERED_OK_ACC : <no caching attributes>
> 
> Sincerely,
> Michael Tsymbalyuk
> 
> > On Tue, 2011-05-10 at 18:08 +0400, Michael Tsymbalyuk wrote:
> > > Thiw webrev fixes the problem of improper handling of page table
> > > attributes related to caching while allocating a DMA-compatible
> > > memory via ddi_dma_mem_alloc(). The patch adds extra attribute check
> > > to ddi_dma_mem_alloc().
> > > 
> > > http://cr.illumos.org/view/asjt99s5/illumos-534-webrev/
> > > 
> > > Sincerely,
> > > Michael Tsymbalyuk
> > > 
> > 
> > This looks good if it is that simple.  I want to do some deeper analysis
> > to review your code.  Out of curiosity, did you test this at all?  If
> > so, how?
> > 
> > 	- Garrett
> > > 
> > > _______________________________________________
> > > Developer mailing list
> > > Developer at lists.illumos.org
> > > http://lists.illumos.org/m/listinfo/developer
> > 
> > 
> 
>