Re: [RFC v5 23/28] hw/pci: Add sec-sid property to PCIDevice

[Eric Auger <eric.auger@redhat.com>]

On 9/3/26 6:07 PM, Tao Tang wrote:
> Hi Eric,
>
> On 2026/8/31 14:03, Eric Auger wrote:
>> Hi Tao,
>>
>> On 8/13/26 6:21 PM, Tao Tang wrote:
>>> Arm SMMUv3 uses a SEC_SID (StreamID Security state) to determine the
>>> security state of the programming interface that controls a
>>> transaction.
>>> The architecture explicitly states that the association between a
>>> device
>>> and its SEC_SID is a system-defined property, not something derived
>>> from
>>> the physical address space.
>>>
>>> We need a way to represent this system-defined SEC_SID for PCI devices
>>> if we want to implement SMMU's Secure state. So that SMMUv3 can select
>>> the correct register bank and configuration when handling their
>>> streams.
>>>
>>> This patch adds a new char *sec_sid field to PCIDevice, together with
>>> a "sec-sid" QOM property. The property is intended to carry the
>>> platform-defined SEC_SID for the device; for now only Non-secure and
>>> Secure security states are supported.
>> You shall add PCI maintainers in cc.
>>
>> Is it only SMMU intended? I mean if you set this property, the PCI
>> device shall send DMA requests with secure attributes too?
>
>
> Yes, the current consumer is Arm SMMUv3 only. The property represents
> the system-defined SEC_SID presented to the SMMU, selecting its
> programming interface and Stream table. It does not make the PCI
> function issue DMA with Secure MemTxAttrs; that per-transaction
> attribute is passed separately through attrs_to_index(). Some code
> pieces: 
yes you're right. Those are different. But how do you tell a device to
send DMA transactions with attrs.secure (-> iommu_idx). That's what I am
confused about. Do we plan to have a prop for that or will it be
hardcoded such as in hw/misc/tz-mpc.c?

>
> ```
> static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
>                                       IOMMUAccessFlags flag, int
> iommu_idx)
> {
> .....
>     SMMUv3State *s = sdev->smmu;
>     SMMUSecSID sec_sid = sdev->sec_sid;
>     SMMUv3RegBank *bank = smmuv3_bank(s, sec_sid);
> .....
>     entry.target_as = smmuv3_bypass_target_as(s, sec_sid, iommu_idx);
> ......
>     if (!smmu_enabled(s, sec_sid)) {
>         if (FIELD_EX32(bank->gbpa, GBPA, ABORT)) {
>             status = SMMU_TRANS_ABORT;
>         } else {
>             status = SMMU_TRANS_DISABLE;
>         }
>         goto epilogue;
>     }
> .....
> }
> ```
>
> We use the device-inherent `PCIDevice->sec_sid` to select the SMMU
> Programming Interface. The actual per-transaction attribute, on the
> other hand, is passed to `smmuv3_translate()` through the `int
> iommu_idx` parameter for each transaction.
>
> And Michael is already included in To: for both the cover letter and
> this patch. I agree that this needs PCI maintainer review. 
OK fine, sorry fo the noise. 

Eric
>
> @Michael, Could you please help review this patch?
>
> Thanks,
> Tao
>
>
>>
>> Eric
>>> Signed-off-by: Tao Tang <tangtao1634@phytium.com.cn>
>>> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>> ---
>>>   hw/pci/pci.c                | 7 +++++++
>>>   include/hw/pci/pci_device.h | 3 +++
>>>   2 files changed, 10 insertions(+)
>>>
>>>
>