On 2/16/23 14:58, Mostafa Saleh wrote:
> Hi Eric,
>
> On Thu, Feb 16, 2023 at 12:56:52PM +0100, Eric Auger wrote:
>>> @@ -1174,14 +1174,35 @@ static int smmuv3_cmdq_consume(SMMUv3State *s)
>>> case SMMU_CMD_TLBI_NH_VA:
>>> smmuv3_s1_range_inval(bs, &cmd);
>>> break;
>>> + case SMMU_CMD_TLBI_S12_VMALL:
>>> + uint16_t vmid = CMD_VMID(&cmd);
>>> +
>>> + if (!STAGE2_SUPPORTED(s->features)) {
>> if you add such checks for S2, may you should consider adding similar
>> ones for existing S1?
> Yes, I will go through the other commands and do the same for stage-1
> only commands.
>
>>> + smmu_inv_notifiers_all(&s->smmu_state);
>>> + smmu_iotlb_inv_vmid(bs, vmid);
>>> + break;
>>> + case SMMU_CMD_TLBI_S2_IPA:
>>> + if (!STAGE2_SUPPORTED(s->features)) {
>>> + cmd_error = SMMU_CERROR_ILL;
>>> + break;
>>> + }
>>> + /*
>>> + * As currently only either s1 or s2 are supported
>>> + * we can reuse same function for s2.
>>> + */
>>> + smmuv3_s1_range_inval(bs, &cmd);
>> Shouldn't we rename the function then?
> I guess we can rename it smmuv3_s1_s2_range_inval, we will have to
> revisit this when nesting is supported.
or simply smmuv3_range_inval, adding a comment specifying its is usable
for both stages
Eric
>
> Thanks,
> Mostafa
>
