qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH for-5.0 v11 08/20] virtio-iommu: Implement translate


From: Auger Eric
Subject: Re: [PATCH for-5.0 v11 08/20] virtio-iommu: Implement translate
Date: Wed, 8 Jan 2020 17:55:52 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0

Hi Jean-Philippe, Peter,

On 1/7/20 11:10 AM, Jean-Philippe Brucker wrote:
> On Mon, Jan 06, 2020 at 12:58:50PM -0500, Peter Xu wrote:
>> On Mon, Jan 06, 2020 at 06:06:34PM +0100, Jean-Philippe Brucker wrote:
>>> On Fri, Dec 20, 2019 at 11:51:00AM -0500, Peter Xu wrote:
>>>> On Fri, Dec 20, 2019 at 05:26:42PM +0100, Jean-Philippe Brucker wrote:
>>>>> There is at the virtio transport level: the driver sets status to
>>>>> FEATURES_OK once it accepted the feature bits, and to DRIVER_OK once its
>>>>> fully operational. The virtio-iommu spec says:
>>>>>
>>>>>   If the driver does not accept the VIRTIO_IOMMU_F_BYPASS feature, the
>>>>>   device SHOULD NOT let endpoints access the guest-physical address space.
>>>>>
>>>>> So before features negotiation, there is no access. Afterwards it depends
>>>>> if the VIRTIO_IOMMU_F_BYPASS has been accepted by the driver.
>>>>
>>>> Before enabling virtio-iommu device, should we still let the devices
>>>> to access the whole system address space?  I believe that's at least
>>>> what Intel IOMMUs are doing.  From code-wise, its:
>>>>
>>>>     if (likely(s->dmar_enabled)) {
>>>>         success = vtd_do_iommu_translate(vtd_as, vtd_as->bus, 
>>>> vtd_as->devfn,
>>>>                                          addr, flag & IOMMU_WO, &iotlb);
>>>>     } else {
>>>>         /* DMAR disabled, passthrough, use 4k-page*/
>>>>         iotlb.iova = addr & VTD_PAGE_MASK_4K;
>>>>         iotlb.translated_addr = addr & VTD_PAGE_MASK_4K;
>>>>         iotlb.addr_mask = ~VTD_PAGE_MASK_4K;
>>>>         iotlb.perm = IOMMU_RW;
>>>>         success = true;
>>>>     }
>>>>
>>>> From hardware-wise, an IOMMU should be close to transparent if you
>>>> never enable it, imho.
>>>
>>> For hardware that's not necessarily the best choice. As cited in my
>>> previous reply it has been shown to introduce vulnerabilities since
>>> malicious devices can DMA during boot, before the OS takes control of the
>>> IOMMU. The Arm SMMU allows an implementation to adopt a deny policy by
>>> default.
>>
>> I see.  But then how to read a sector from the block to at least boot
>> an OS if we use a default-deny policy?  Does it still need a mapping
>> that is established somehow by someone before hand?
> 
> Yes, it looks like EDK II uses IOMMU operations in order to access those
> devices on platforms where the IOMMU isn't default-bypass (AMD SEV support
> is provided by edk2, and a VT-d driver seems provided by edk2-platforms).
> However for OVMF we could just set the bypass feature bit in virtio-iommu
> device, which doesn't even requires setting up the virtqueue.
> 
> I'm missing a piece of the puzzle for Arm platforms though, because it
> looks like Trusted Firmware-A sets up the default-deny policy on reset
> even when it wasn't hardwired, but doesn't provide a service to create
> SMMUv3 mappings for the bootloader.
> 
> Thanks,
> Jean
> 

I think we have a concrete example for the above discussion. The AHCI.
When running the virtio-iommu on x86 I get messages like:

virtio_iommu_translate sid=250 is not known!!
no buffer available in event queue to report event

and a bunch of "AHCI: Failed to start FIS receive engine: bad FIS
receive buffer address" messages (For each port)

This was reported in my cover letter (*). This happens very early in the
boot process, before the OS get the hand and before the virtio-iommu
driver creates any mapping. It does not prevent the guest from booting
though.

Currently the virtio-iommu device checks the VIRTIO_IOMMU_F_BYPASS. If I
overwrite it to true in the device, then, the guest boots without those
messages.

I share Peter's concern about having a different default policy than x86.

Thanks

Eric

Note the migration issue reported in the cover letter is fixed now and
was due to the migration priority unset.






reply via email to

[Prev in Thread] Current Thread [Next in Thread]