qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Peter Xu <peterx@redhat.com>
To: David Kiarie <davidkiarie4@gmail.com>, alexaltea123@gmail.com
Cc: Jan Kiszka <jan.kiszka@siemens.com>,
	QEMU Developers <qemu-devel@nongnu.org>,
	Valentine Sinitsyn <valentine.sinitsyn@gmail.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	rkrcmar@redhat.com
Subject: Re: [Qemu-devel] Fwd: QEMU: AMD IOMMU implementation bugs
Date: Mon, 18 Feb 2019 16:22:17 +0800	[thread overview]
Message-ID: <20190218082217.GA9040@xz-x1> (raw)
In-Reply-To: <CABdVeAAvoXr5oG+G8NYJDiNO4Tdgu2QNhR4dVzd_wZ=W2GhgPg@mail.gmail.com>

On Sat, Feb 16, 2019 at 10:11:28PM +0300, David Kiarie wrote:
> ---------- Forwarded message ---------
> From: Alexandro Sánchez Bach <alexaltea123@gmail.com>
> Date: Wed, Jan 31, 2018 at 2:29 AM
> Subject: QEMU: AMD IOMMU implementation bugs
> To: <davidkiarie4@gmail.com>
> 
> 
> 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

  parent reply	other threads:[~2019-02-18  8:32 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CACQB2UxS6a2LbnOsdLt6q1=9xN4NoGBQNwVNZe-vGQjWdus_KQ@mail.gmail.com>
2019-02-16 19:11 ` [Qemu-devel] Fwd: QEMU: AMD IOMMU implementation bugs David Kiarie
2019-02-16 19:35   ` [Qemu-devel] " David Kiarie
2019-02-18  8:22   ` Peter Xu [this message]
2019-02-18 18:59     ` [Qemu-devel] Fwd: " David Kiarie
2019-02-22 15:34       ` David Kiarie

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190218082217.GA9040@xz-x1 \
    --to=peterx@redhat.com \
    --cc=alexaltea123@gmail.com \
    --cc=davidkiarie4@gmail.com \
    --cc=jan.kiszka@siemens.com \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=rkrcmar@redhat.com \
    --cc=valentine.sinitsyn@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).