Re: [PATCH V3 05/11] accel/amdxdna: Add hardware context

[Jeffrey Hugo]

On 10/6/2024 11:02 PM, Lizhi Hou wrote:
> On 10/4/24 10:50, Jeffrey Hugo wrote:
> > On 9/11/2024 12:05 PM, Lizhi Hou wrote:
> > > diff --git a/drivers/accel/amdxdna/aie2_ctx.c 
> > > b/drivers/accel/amdxdna/aie2_ctx.c
> > > new file mode 100644
> > > index 000000000000..52a71661f887
> > > --- /dev/null
> > > +++ b/drivers/accel/amdxdna/aie2_ctx.c
> > > @@ -0,0 +1,186 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Copyright (C) 2024, Advanced Micro Devices, Inc.
> > > + */
> > > +
> > > +#include <drm/amdxdna_accel.h>
> > > +#include <drm/drm_device.h>
> > > +#include <drm/drm_print.h>
> > > +#include <linux/types.h>
> > > +
> > > +#include "amdxdna_ctx.h"
> > > +#include "amdxdna_mailbox.h"
> > > +#include "amdxdna_pci_drv.h"
> > > +#include "aie2_pci.h"
> > > +#include "aie2_solver.h"
> >
> > Alphabetical order?
> >
> > <snip>
> > > +/**
> > > + * struct amdxdna_drm_create_hwctx - Create hardware context.
> > > + * @ext: MBZ.
> > > + * @ext_flags: MBZ.
> > > + * @qos_p: Address of QoS info.
> > > + * @umq_bo: BO handle for user mode queue(UMQ).
> > > + * @log_buf_bo: BO handle for log buffer.
> > > + * @max_opc: Maximum operations per cycle.
> > > + * @num_tiles: Number of AIE tiles.
> > > + * @mem_size: Size of AIE tile memory.
> > > + * @umq_doorbell: Returned offset of doorbell associated with UMQ.
> > > + * @handle: Returned hardware context handle.
> > > + * @pad: MBZ.
> >
> > You specify must be zero, but I don't see that checked anywhere.
>
> Ok. I did not see any driver checks structure pad. And pad should not be 
> used. I will remove MBZ for pad.

Be aware, if you don't check for 0, you can never reuse that portion of 
the structure for future use as there may be existing userspace that 
happens to be putting random garbage in that area at that future point.

-Jeff