Re: [RFC PATCH v2 08/11] hw/arm/smmuv3: Add CMDs related to stage-2

[Mostafa Saleh <smostafa@google.com>]

Hi Eric,

On Mon, Mar 20, 2023 at 05:51:07PM +0100, Eric Auger wrote:
> Hi Mostafa,
> 
> On 2/26/23 23:06, Mostafa Saleh wrote:
> > CMD_TLBI_S2_IPA: As S1+S2 is not enabled, for now this can be the
> > same as CMD_TLBI_NH_VAA.
> >
> > CMD_TLBI_S12_VMALL: Added new function to invalidate TLB by VMID.
> >
> > For stage-1 only commands, add a check to to throw CERROR_ILL if used
> s/to to/to
Will do.

> > @@ -1211,12 +1211,22 @@ static int smmuv3_cmdq_consume(SMMUv3State *s)
> >          {
> >              uint16_t asid = CMD_ASID(&cmd);
> >  
> > +            if (!STAGE1_SUPPORTED(s)) {
> > +                cmd_error = SMMU_CERROR_ILL;
> Well looking further this is not said explicitly this should return
> 
> SMMU_CERROR_ILL. Maybe you should mark it as a guest error because we do not expect a guest to send such command when S1 is not supported?
> 
I can add a check after the switch for SMMU_CERROR_ILL to log a guest
error. 
> > +                break;
> > +            }
> > +
> >              trace_smmuv3_cmdq_tlbi_nh_asid(asid);
> >              smmu_inv_notifiers_all(&s->smmu_state);
> >              smmu_iotlb_inv_asid(bs, asid);
> >              break;
> >          }
> >          case SMMU_CMD_TLBI_NH_ALL:
> > +            if (!STAGE1_SUPPORTED(s)) {
> > +                cmd_error = SMMU_CERROR_ILL;
> > +                break;
> 
> there is a VMID field. Can't this be used in S2 mode as well?
According to the user manual "4.4.2 TLB invalidation of stage 1"
CMD_TLBI_NH_ALL causes CERROR_ILL if stage-1 is not supported.

Thanks,
Mostafa