Re: [PATCH v4 18/19] hw/arm/smmuv3: Advertise S2FWB

[Mostafa Saleh <smostafa@google.com>]

Hi Eric,

On Mon, Jul 08, 2024 at 07:09:02PM +0200, Eric Auger wrote:
> Hi Mostafa,
> 
> On 7/4/24 20:36, Jean-Philippe Brucker wrote:
> > On Mon, Jul 01, 2024 at 11:02:40AM +0000, Mostafa Saleh wrote:
> >> QEMU doesn's support memory attributes, so FWB is NOP, this
> >> might change in the future if memory attributre would be supported.
> attributes here and below as reported along with v3
> >>
> >> Signed-off-by: Mostafa Saleh <smostafa@google.com>
> >> ---
> >>  hw/arm/smmuv3.c | 8 ++++++++
> >>  1 file changed, 8 insertions(+)
> >>
> >> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> >> index 807f26f2da..88378e83dd 100644
> >> --- a/hw/arm/smmuv3.c
> >> +++ b/hw/arm/smmuv3.c
> >> @@ -287,6 +287,14 @@ static void smmuv3_init_regs(SMMUv3State *s)
> >>      if (FIELD_EX32(s->idr[0], IDR0, S2P)) {
> >>          /* XNX is a stage-2-specific feature */
> >>          s->idr[3] = FIELD_DP32(s->idr[3], IDR3, XNX, 1);
> >> +        if (FIELD_EX32(s->idr[0], IDR0, S1P)) {
> > Why is this check needed?
> >
> >> +            /*
> >> +             * QEMU doesn's support memory attributes, so FWB is NOP, this
> > doesn't
> I have just seen your reply on my v3 comments. I still do not understand
> why we expose this bit at this stage.

As I replied to Jean, I will drop this patch for now, we can always add it later,
as it doens't add much value.

Thanks,
Mostafa

> 
> Thanks
> 
> Eric
> >
> > Thanks,
> > Jean
> >
> >> +             * might change in the future if memory attributre would be
> >> +             * supported.
> >> +             */
> >> +           s->idr[3] = FIELD_DP32(s->idr[3], IDR3, FWB, 1);
> >> +        }
> >>      }
> >>      s->idr[3] = FIELD_DP32(s->idr[3], IDR3, RIL, 1);
> >>      s->idr[3] = FIELD_DP32(s->idr[3], IDR3, BBML, 2);
> >> -- 
> >> 2.45.2.803.g4e1b14247a-goog
> >>
>