All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Xu <peterx@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: Eduardo Habkost <eduardo@habkost.net>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	Jason Wang <jasowang@redhat.com>,
	wei.huang2@amd.com,
	Richard Henderson <richard.henderson@linaro.org>,
	QEMU Developers <qemu-devel@nongnu.org>,
	Suravee.Suthikulpanit@amd.com,
	Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: who's maintaining amd_iommu.c these days?
Date: Thu, 31 Mar 2022 14:30:38 -0400	[thread overview]
Message-ID: <YkXzToltd73X4WgY@xz-m1.local> (raw)
In-Reply-To: <CAFEAcA_17Mzz7AiQd+1z50Jp-wWhfChCVi=8kjWCU6daUVqV_Q@mail.gmail.com>

On Thu, Mar 31, 2022 at 05:01:52PM +0100, Peter Maydell wrote:
> Coverity points out some problems with hw/i386/amd_iommu.c's event
> logging code -- specifically, CID 1487115 1487116 1487190 1487200
> 1487232 1487258 are all the same basic problem, which is that various
> functions declare a local "uint64_t evt[4]", populate only some
> bits of it and then write it to guest memory, so we end up using
> uninitialized host data and leaking it to the guest. I was going to
> write a fix for this, but in looking at the code I noticed that
> it has more extensive problems:
> 
> (1) these functions allocate an array of 4 64-bit values,
> but we only copy 2 to the guest, because AMDVI_EVENT_LEN is 16.
> Looking at the spec, I think that the length is right and it's
> really 4 32-bit values (or 2 64-bit values, if you like).
> 
> (2) There are host-endianness bugs, because we assemble the
> event as a set of host-endianness values but then write them
> to guest memory as a bag-of-bytes with dma_memory_write()
> 
> (3) amdvi_encode_event() is throwing away most of its
> "addr" argument, because it calls
>   amdvi_setevent_bits(evt, addr, 63, 64) apparently intending
> that to write 64 bits starting at 63 bits into the packet, but
> the amdvi_setevent_bits() function only ever updates one
> uint64_t in the array, so it will in fact write bit 63 and
> nothing else.
> 
> (4) The claimed bit layout of the event structure doesn't
> match up with the one in the spec document I found. This
> could be because I found a document for some other bit
> of hardware, of course.
> 
> Anyway, adding all these up, the event logging probably
> needs a bit of a restructuring, and that should ideally be
> done by somebody who (a) knows the hardware we're emulating
> here and (b) is in a position to test things. Any volunteers?

Copying some AMD developers (from where I saw the last patches from)...

-- 
Peter Xu



  reply	other threads:[~2022-03-31 18:48 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-31 16:01 who's maintaining amd_iommu.c these days? Peter Maydell
2022-03-31 18:30 ` Peter Xu [this message]
2022-04-01  2:09   ` Jason Wang
2022-04-01 16:11     ` Wei Huang
2022-04-01 16:14       ` Peter Maydell
2022-04-01 16:35         ` Wei Huang

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=YkXzToltd73X4WgY@xz-m1.local \
    --to=peterx@redhat.com \
    --cc=Suravee.Suthikulpanit@amd.com \
    --cc=eduardo@habkost.net \
    --cc=jasowang@redhat.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    --cc=wei.huang2@amd.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.