Re: [Qemu-devel] [V9 0/4] AMD IOMMU

[David Kiarie]

On Wed, May 4, 2016 at 2:05 PM, Valentine Sinitsyn
<address@hidden> wrote:
> On 04.05.2016 16:02, David Kiarie wrote:
>>
>>
>>
>> On 04/05/16 13:58, Valentine Sinitsyn wrote:
>>>
>>> On 04.05.2016 15:51, David Kiarie wrote:
>>>>
>>>> On Wed, May 4, 2016 at 10:39 AM, Valentine Sinitsyn
>>>> <address@hidden> wrote:
>>>>>
>>>>> Hi everyone,
>>>>>
>>>>> On 04.05.2016 12:05, David Kiarie wrote:
>>>>>>
>>>>>>
>>>>>> On Wed, May 4, 2016 at 9:12 AM, Jan Kiszka <address@hidden> wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 2016-04-30 00:42, David Kiarie wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> These series adds AMD IOMMU support to Qemu. It's currently in
>>>>>>>> the 9th
>>>>>>>> version.
>>>>>>>>
>>>>>>>> In this series I have (hopefully) addressed all the comments made
>>>>>>>> in the
>>>>>>>> previous version.
>>>>>>>> I have also tested and successfully passed-through PCI device 'ac97'
>>>>>>>> with more devices to be tested.
>>>>>>>>
>>>>>>>
>>>>>>> I've done some basic testing with a Jailhouse setup and found it
>>>>>>> working. The ACPI table is now properly parsed and the DMA
>>>>>>> remapping was
>>>>>>> not disturbing the system after Jailhouse was activated.
>>>>>>>
>>>>>>> However, it was also still not intervening after I started to corrupt
>>>>>>> the configuration, removed DMA target properties from most of the
>>>>>>> RAM or
>>>>>>> dropped PCI devices.
>>>>>
>>>>>
>>>>> Please also remember that unlisted devices go without translation.
>>>>> To "mute"
>>>>> the device, set V, TV, the DomainId, and zero everything else in the
>>>>> DTE.
>>>>>
>>>>>>
>>>>>> This means you're invalidating DTEs ?
>>>>>>
>>>>>>>
>>>>>>> You are not dropping invalid remapping requests, are you?
>>>>>>> According to
>>>>>>> the logs, you are detecting them at least:
>>>>>>>
>>>>>>> (amd-iommu)amd_iommu_get_dte: Device Table at 0x3b0d4000
>>>>>>> (amd-iommu)amd_iommu_get_dte: Pte entry at 0x0 is invalid
>>>>>>> (amd-iommu)amd_iommu_translate: devid: 00:02.0 gpa 0x32f39480 hpa
>>>>>>> 0x32f39000
>>>>>>>
>>>>>>> It's a bit hard to test right now if remapping is actually properly
>>>>>>> working in all important cases if you do not reject invalid ones.
>>>>>
>>>>>
>>>>> My understanding is that you should generate an IO_PAGE_FAULT event
>>>>> and drop
>>>>> the request. This doesn't apply to ATS, which is a bit trickier, but we
>>>>> don't address ATS in this patch series anyway, do we?
>>>>
>>>>
>>>> My next question is what you mean by 'reject' and 'drop'. In I
>>>> encounter an invalid PTE/DTE I don't translate the gpa, it just become
>>>> the hpa which is what is happening above.
>>>
>>> What happens if you just ignore the request? I mean, what if you don't
>>> forward it to anywhere else in QEMU, just log this event and return?
>>
>>
>> Am guessing this should have something to do with pci abort which, last
>> time I tried, wasn't aborting at request. I will look at it again.
>
> My initial answer was also "do target abort". But then I did a quick look
> over the spec, and found no such requirement. Please read relevant parts
> thoroughly yourself, and maybe experiment with "just ignore"/"explicitly
> abort" options.

Qemu doesn't seem to be honouring target aborts. The PCI device
represented by IOMMU seems like just a link to allow communication
with the OS/System Software.

In place of target aborts I am going to instead populate the struct
(IOMMUTLBEntry) like below

  IOMMUTLBEntry ret = {
        .target_as = &address_space_memory,
        .iova = addr,
        .translated_addr = 0,
        .addr_mask = ~(hwaddr)0,
        .perm = IOMMU_NONE,
    }

Functionally speaking, this doesn't seem very different from a target
abort because with a target abort, the bus is expected to complete the
translation which should come down to the same thing(with the later
being more complicated).

On the other hand, I am yet to confirm, from the spec that the
particular reported case warrants a target abort but cases such as
page faults should be target aborted (which is not currently the case,
so they should be fixed).


>
> Valentine