Re: [PATCH v3 1/7] memory: associate DMA accesses with the initiator Device

[Peter Maydell]

On Tue, 15 Nov 2022 at 16:20, Peter Xu <peterx@redhat.com> wrote:
>
> On Fri, Oct 28, 2022 at 03:16:42PM -0400, Alexander Bulekov wrote:
> > +    /* Do not allow more than one simultanous access to a device's IO 
> > Regions */
> > +    if (mr->owner &&
> > +            !mr->ram_device && !mr->ram && !mr->rom_device && 
> > !mr->readonly) {
> > +        dev = (DeviceState *) object_dynamic_cast(mr->owner, TYPE_DEVICE);
> > +        if (dev->mem_reentrancy_guard.engaged_in_io) {
>
> Do we need to check dev being non-NULL?  Fundamentally it's about whether
> the owner can be not a DeviceState, I believe it's normally true but I
> can't tell; at least from memory region API it can be any Object*.

There is at least one MemoryRegion in the tree whose owner is not
a DeviceState: hw/arm/virt.c does:

        memory_region_init(secure_sysmem, OBJECT(machine), "secure-memory",
                           UINT64_MAX);

and MachineState inherits directly from Object, not via DeviceState.

More generally, when doing a QOM cast, either:
 (1) you know that the object must be of the right type, in which
     case you should use the cast macro (which will assert for you), or
 (2) the object may not be of the right type, in which case you
     use object_dynamic_cast() and check whether it returned NULL

The combination of object_dynamic_cast() and no NULL check is I
think usually a bug.

-- PMM