[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] memory region: check the old.mmio.read status
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH] memory region: check the old.mmio.read status |
Date: |
Thu, 13 Sep 2018 01:31:29 +0100 |
On 12 September 2018 at 18:43, Laszlo Ersek <address@hidden> wrote:
> On 09/12/18 14:54, Peter Maydell wrote:
>> There's patches on-list which drop the old_mmio field from the MemoryRegion
>> struct entirely, so I think this patch as it stands is obsolete.
>>
>> Currently our semantics are "you must provide both read and write, even
>> if one of them just always returns 0 / does nothing / returns an error".
>
> That's new to me. Has this always been the case?
Pretty sure it has, yes, because the code assumes that if you can
get a guest read then your MemoryRegion provides an accessor for it.
If your guest never actually tries to do a read then of course we'll
never notice...
> There are several
> static MemoryRegionOps structures that don't conform. (See the end of my
> other email:
> <http://mid.mail-archive.com/address@hidden>.)
> Beyond the one that Li Qiang reported directly ("fw_cfg_ctl_mem_read").
>
> Are all of those ops guest-triggerable QEMU crashers?
Some of them are special cases like the notdirty-memory one where
reads always go to host RAM rather than taking the slow path via
the io accessor. But others are probably guest crashers.
>> We could probably reasonably assert this at the point when the
>> MemoryRegionOps is registered.
>
> Apparently, we should have...
Yeah. Or we could define a default for if there's no read function,
which I guess should be the same as what we do if
memory_region_access_valid() fails. If we want that then the
simplest thing is for memory_region_access_valid() itself to
check that at least one of the accessor functions exists and
return false if none do. (But as I mention above we should get
all the "old_mmio is going away" patches in first and base the
change on that, or there'll be a conflict.)
thanks
-- PMM