Re: [Qemu-devel] Fwd: QEMU: AMD IOMMU implementation bugs

[Peter Xu]

On Sat, Feb 16, 2019 at 10:11:28PM +0300, David Kiarie wrote:
> ---------- Forwarded message ---------
> From: Alexandro Sánchez Bach <address@hidden>
> Date: Wed, Jan 31, 2018 at 2:29 AM
> Subject: QEMU: AMD IOMMU implementation bugs
> To: <address@hidden>
> 
> 
> Hey David,
> 
> hello Mr Alexandro Sanchez Bach,
> 
> I'm working with your AMD IOMMU implementation since I'm writing a
> PlayStation 4 emulator. I found few bugs, that I wanted to report to you.
> 
> 1. https://github.com/qemu/qemu/blob/master/hw/i386/amd_iommu.c#L380
> Did you mean amdvi_assign_orq instead of  amdvi_test_mask? Otherwise my
> guest OS will get stuck in an endless loop waiting for a flag that never
> comes.
> 
> it looks like you're correct to me.
> 
> 2. The arguments `addr` and `val` are swapped in these two lines:
> > static void amdvi_writeq_raw(AMDVIState *s, uint64_t val, hwaddr addr);
> > amdvi_writeq_raw(s, addr, amdvi_readq(s, addr) | val);
> 
> same here.
> 
> 3. And this might be something specific to my AMD IOMMU (1022:1437, Family
> 16h), but the PS4 OS is checks the entire word stored in these registers
> and computes the expression as:
> config[AMDVI_CAPAB_BAR_LOW] | cap[AMDVI_CAPAB_BAR_HIGH] << 32
> So just storing half words should be wrong.
> 
> not sure about this.
> 
> since it looks to me like you're working with emulation and virtualization,
> i have cc'd people who i know/knew to be actively involved. it doubt it
> would take any effort for these people to respond to bugs like above in the
> future.

Hi, Alex,

The sentences are a bit messed up above.  IMHO you can simply post
patches directly if you found any bugs in the code.  You can prefix
the subject with "RFC" if you are uncertain about the changes.  People
on the list can directly comment on the patches.

Regards,

-- 
Peter Xu