Re: [PATCH v3 1/2] intel_iommu: fix guest-triggerable abort on oversized MMIO access

[Junjie Cao <junjie.cao@intel.com>]

Hi Yi and Zhenzhong,

Thanks both.

On Yi's question -- I enumerated the 25 v2 assert sites and checked
offset & 7.  21 (3 reads + 18 writes) sit at non-8-aligned offsets
and are unreachable (memory_region_access_valid() rejects 8-byte
access there) -- just deleted.  The remaining 4, all writes at
8-aligned offsets, are reachable: FECTL 0x38, IECTL 0xa0,
IEADDR 0xa8, PECTL 0xe0.

(FEADDR 0x40 is 8-aligned too but already takes 8-byte writes via
vtd_set_quad() for Xen; GCMD 0x18 and VER 0x0 never asserted on
size and their default/long paths already truncate.)

On the log -- Yi, fair point that the truncation is harmless from
QEMU's side.  The one thing I'd gently float: the VT-d spec is
silent on oversized accesses to 32-bit registers, so the guest is
in undefined territory, and Zhenzhong's LOG_GUEST_ERROR suggestion
is free unless -d guest_errors is passed.  If that reasoning works
for you, I'd combine both -- keep the log (with Zhenzhong's API)
and add the comment you asked for, so future maintainers don't
delete the block as "harmless":

    /*
     * 32-bit register at an 8-byte-aligned offset: a well-formed
     * 8-byte guest access reaches this handler.  vtd_set_long()
     * takes uint32_t and truncates the high half -- undefined
     * per the VT-d spec but harmless here.  Flag it so
     * -d guest_errors surfaces the guest-side bug.
     */
    if (size != 4) {
        qemu_log_mask(LOG_GUEST_ERROR,
                      "%s: invalid %u-byte access to 32-bit reg "
                      "addr=0x%" PRIx64 "\n", __func__, size, addr);
    }
    vtd_set_long(s, addr, val);
    vtd_handle_*_write(s);

Happy either way -- if you'd still rather drop the log, I'll do
that in v4.

Thanks,
Junjie