Re: [Xen-devel] [PATCH v16 4/9] x86: add multiboot2 protocol support for EFI platforms

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 09/03/17 15:24, Daniel Kiper wrote:
> On Tue, Mar 07, 2017 at 12:39:04AM +0100, Daniel Kiper wrote:
>> On Wed, Feb 22, 2017 at 09:04:17AM -0800, Doug Goldstein wrote:
>>
>> [...]
>>
>>> I'm currently at ELC and then on vacation so I don't have access to any
>>> of the machines currently myself. However the machine I most use to test
>>> is a NUC5i5MYHE and a NUC5i3MYHE if you want to ask around if someone
>>> has one internally. But that's why I gave QEMU as an example.
>>>
>>> I was using qemu master from a few weeks ago. I'll have to find the
>>> revision for you. But the command line I use is:
>>>
>>> -enable-kvm -M pc-q35-2.8 -device intel-iommu -cpu host -m 2048 -smp 2
>>> -drive if=pflash,format=raw,file=/tmp/tmp.EiR6ixmYzV -global
>>> isa-debugcon.iobase=0x402 -debugcon file:/tmp/tmp.nuvEXUWfnA -monitor
>>> stdio -chardev socket,host=127.0.0.1,port=25914,id=S0,server,nowait
>>> -device isa-serial,chardev=S0 -device piix3-usb-uhci -device usb-tablet
>>> -netdev id=net0,type=tap -device
>>> virtio-net-pci,netdev=net0,mac=52:54:00:12:34:56 -boot order=n -device
>>> qxl-vga -gdb tcp::14952
>> Sadly, my colleagues and I are not able to reproduce the problem on any of
>> machines available for us (available on the market and some development
>> stuff in our labs). I did tests with QEMU (I am not able to run it with
>> "-device intel-iommu" on my machine; I have to investigate this). Everything
>> works. Joao did some tests on Intel NUC D34010WYK second generation.
>> Everything works. So, Konrad ordered Intel NUC NUC5i3MYHE for me. I am
>> waiting for delivery. Doug, could you tell me what distro, Xen, etc. you
>> have installed on that NUC? I would like to test same config as yours on
>> this machine.
> Latest QEMU from git with intel-iommu device enabled works without any issue.
> Though it looks that I found a bug in Xen IOMMU code. If I run Xen from master
> in QEMU then QEMU complains and crashes:
>
>   qemu-system-x86_64: /srv/dev/qemu/qemu_20170308/hw/i386/intel_iommu.c:1786: 
> vtd_mem_write: Assertion `size == 4' failed.
>   Aborted (core dumped)
>
> So, I took a look at dma_msi_set_affinity() and found this:
>
>   dmar_writeq(iommu->reg, DMAR_FEADDR_REG, msg.address);
>
> It looks bogus because DMAR_FEADDR_REG is defined in spec as 32-bit wide.
> So, applied this patch:
>
> -    dmar_writeq(iommu->reg, DMAR_FEADDR_REG, msg.address);
> +    dmar_writel(iommu->reg, DMAR_FEADDR_REG, msg.address_lo);
> +    if (x2apic_enabled)
> +        dmar_writel(iommu->reg, DMAR_FEUADDR_REG, msg.address_hi);

EFI aside, that is clearly a qemu bug (as Xen shouldn't be able to cause
an assertion like that to be hit), and there is nothing wrong with Xens
code as-is.

All the spec requires is that 32bit fields are accessed as whole
doublewords (rather than smaller units).  There is no restriction on
updating two adjacent 32bit fields with a quadword access. 
(Furthermore, hardware is requires to cope with accesses to a 64bit
field being decomposed into two 32bit accesses.)

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel