Jason Gunthorpe <[email protected]> writes:

>> > static inline dma_addr_t dma_direct_map_phys(struct device *dev,
>> >            phys_addr_t phys, size_t size, enum dma_data_direction dir,
>> >            unsigned long attrs, bool flush)
>> > {
>> >    dma_addr_t dma_addr;
>> > 
>> >    if (is_swiotlb_force_bounce(dev)) {
>> >            if (attrs & (DMA_ATTR_MMIO | DMA_ATTR_REQUIRE_COHERENT))
>> >                    return DMA_MAPPING_ERROR;
>> > 
>> >            return swiotlb_map(dev, phys, size, dir, attrs);
>> >    }
>> > 
>> >    if (attrs & DMA_ATTR_MMIO) {
>> >            dma_addr = phys;
>> >            if (unlikely(!dma_capable(dev, dma_addr, size, false, attrs)))
>> >                    goto err_overflow;
>> >            goto dma_mapped;
>> 
>> I suspect P2P is probably broken on CC because this doesn't make
>> sense..
>
> Actually, I suppose it is fully broken because it will jump to swiotlb
> and then should fail.
>
>> This should flow into the
>> phys_to_dma_unencrypted/phys_to_dma_encrypted block as well AFAICT, it
>> shouldn't just assign phys. Assigning phys to dma on a CC system is
>> always wrong, right?
>> 
>> It is is more like
>> 
>>         /* To be updated, callers should specify MMIO | CC_SHARED instead of
>>         * implying it. */
>>         if (attrs & DMA_ATTR_MMIO)
>>         attrs |= DMA_ATTR_CC_SHARED;
>
> So no need for this if, we can go directly to marking the MMIO callers
> with DMA_ATTR_CC_SHARED once this is fixed for mmio:
>
>>         if (attrs & DMA_ATTR_CC_SHARED) {
>>              dma_addr = phys_to_dma_unencrypted(dev, phys);
>>      } else {
>>              dma_addr = phys_to_dma_encrypted(dev, phys);
>>      }
>
> Jasn

I am wondering whether this is better

static inline dma_addr_t dma_direct_map_phys(struct device *dev,
                phys_addr_t phys, size_t size, enum dma_data_direction dir,
                unsigned long attrs, bool flush)
{
        dma_addr_t dma_addr;

        /*
         * For a device requiring unencrypted DMA, MMIO memory is treated
         * as shared.
         */
        if (force_dma_unencrypted(dev) && (attrs & DMA_ATTR_MMIO))
                attrs |= DMA_ATTR_CC_SHARED;

.....

        if (attrs & DMA_ATTR_CC_SHARED)
                dma_addr = phys_to_dma_unencrypted(dev, phys);
        else
                dma_addr = phys_to_dma_encrypted(dev, phys);

        if (attrs & DMA_ATTR_MMIO) {
                if (unlikely(!dma_capable(dev, dma_addr, size, false, attrs)))
                        goto err_overflow;
                goto dma_mapped;
        }


....
-aneesh

Reply via email to