On 03/12/2020 07:36, Jan Kiszka wrote:
> On 27.11.20 12:41, Andrea Bastoni wrote:
>> The SMMUv2 allows filtering bits when matching stream IDs before they're
>> passed to the TCU. In this way multiple streams legally get the same
>> translation.
>>
>> On boards such as the ZCU Ultrascale+, the master ID needed to identify
>> the corresponding SMMU stream ID may be dependent on a specific AXI ID
>> that is set by the PL (and could be IP specific).
>>
>> One single fixed mask to pass to the SMR to compact multiple stream IDs
>> before they "hit" the TCU is not flexible enough. The use-case is to
>> compact similar PL-originating masters and have the SMMU behaving the
>> same for them (e.g., they're assigned to the same inmate). At the
>> same time, one needs a full stream_id to assign e.g., different GEM
>> ethernets to different inmates.
>>
>> Update a stream_id to support two different interpretations:
>> - for the SMMUv2, provide an explicit mask + ID.
>> - for the SMMUv3, keep the current single ID.
>>
>> This commit updates the SMMUv2 / v3 --including configuration--
>> accordingly.
>
> CC'ing Nikil and Peng on their affected code.
>
>>
>> Signed-off-by: Andrea Bastoni <[email protected]>
>> ---
>> configs/arm64/imx8qm-linux-demo.c | 7 ++-
>> configs/arm64/imx8qm.c | 16 +++++--
>> configs/arm64/k3-j7200-evm-linux-demo.c | 2 +-
>> configs/arm64/k3-j7200-evm.c | 2 +-
>> configs/arm64/k3-j721e-evm-linux-demo.c | 2 +-
>> configs/arm64/k3-j721e-evm.c | 2 +-
>> configs/arm64/ultra96.c | 11 ++++-
>> configs/arm64/zynqmp-zcu102.c | 15 +++++-
>> hypervisor/arch/arm64/smmu-v3.c | 9 ++--
>> hypervisor/arch/arm64/smmu.c | 64 ++++++++++++++++---------
>> hypervisor/arch/arm64/ti-pvu.c | 21 ++++----
>> include/jailhouse/cell-config.h | 18 ++++---
>> 12 files changed, 114 insertions(+), 55 deletions(-)
>>
>> diff --git a/configs/arm64/imx8qm-linux-demo.c
>> b/configs/arm64/imx8qm-linux-demo.c
>> index f13ca7bc..e8e8b217 100644
>> --- a/configs/arm64/imx8qm-linux-demo.c
>> +++ b/configs/arm64/imx8qm-linux-demo.c
>> @@ -19,7 +19,7 @@ struct {
>> struct jailhouse_memory mem_regions[18];
>> struct jailhouse_irqchip irqchips[4];
>> struct jailhouse_pci_device pci_devices[2];
>> - __u32 stream_ids[1];
>> + union jailhouse_stream_id stream_ids[1];
>> } __attribute__((packed)) config = {
>> .cell = {
>> .signature = JAILHOUSE_CELL_DESC_SIGNATURE,
>> @@ -194,6 +194,9 @@ struct {
>> },
>>
>> .stream_ids = {
>> - 0x10,
>> + {
>> + .mmu500.mask = 0x7f8,
>> + .mmu500.id = 0x10,
>
> Would list id before the mask.
>
>> + },
>> },
>> };
>> diff --git a/configs/arm64/imx8qm.c b/configs/arm64/imx8qm.c
>> index d63c73cf..2ec4f4dd 100644
>> --- a/configs/arm64/imx8qm.c
>> +++ b/configs/arm64/imx8qm.c
>> @@ -20,7 +20,7 @@ struct {
>> struct jailhouse_memory mem_regions[15];
>> struct jailhouse_irqchip irqchips[3];
>> struct jailhouse_pci_device pci_devices[2];
>> - __u32 stream_ids[3];
>> + union jailhouse_stream_id stream_ids[3];
>> } __attribute__((packed)) config = {
>> .header = {
>> .signature = JAILHOUSE_SYSTEM_SIGNATURE,
>> @@ -54,7 +54,6 @@ struct {
>> .type = JAILHOUSE_IOMMU_ARM_MMU500,
>> .base = 0x51400000,
>> .size = 0x40000,
>> - .arm_mmu500.sid_mask = 0x7f80,
>> },
>> },
>>
>> @@ -209,6 +208,17 @@ struct {
>> },
>>
>> .stream_ids = {
>> - 0x11, 0x12, 0x13,
>> + {
>> + .mmu500.mask = 0x7f8,
>> + .mmu500.id = 0x11,
>> + },
>> + {
>> + .mmu500.mask = 0x7f8,
>> + .mmu500.id = 0x12,
>> + },
>> + {
>> + .mmu500.mask = 0x7f8,
>> + .mmu500.id = 0x13,
>> + },
>> },
>> };
>> diff --git a/configs/arm64/k3-j7200-evm-linux-demo.c
>> b/configs/arm64/k3-j7200-evm-linux-demo.c
>> index 90a0ce4c..ace9cd3a 100644
>> --- a/configs/arm64/k3-j7200-evm-linux-demo.c
>> +++ b/configs/arm64/k3-j7200-evm-linux-demo.c
>> @@ -26,7 +26,7 @@ struct {
>> struct jailhouse_memory mem_regions[19];
>> struct jailhouse_irqchip irqchips[3];
>> struct jailhouse_pci_device pci_devices[2];
>> - __u32 stream_ids[1];
>> + union jailhouse_stream_id stream_ids[1];
>> } __attribute__((packed)) config = {
>> .cell = {
>> .signature = JAILHOUSE_CELL_DESC_SIGNATURE,
>> diff --git a/configs/arm64/k3-j7200-evm.c b/configs/arm64/k3-j7200-evm.c
>> index c3ac331d..d0c8aee3 100644
>> --- a/configs/arm64/k3-j7200-evm.c
>> +++ b/configs/arm64/k3-j7200-evm.c
>> @@ -21,7 +21,7 @@ struct {
>> struct jailhouse_memory mem_regions[32];
>> struct jailhouse_irqchip irqchips[6];
>> struct jailhouse_pci_device pci_devices[2];
>> - __u32 stream_ids[1];
>> + union jailhouse_stream_id stream_ids[1];
>> } __attribute__((packed)) config = {
>> .header = {
>> .signature = JAILHOUSE_SYSTEM_SIGNATURE,
>> diff --git a/configs/arm64/k3-j721e-evm-linux-demo.c
>> b/configs/arm64/k3-j721e-evm-linux-demo.c
>> index 5b6aa82e..1b8b3c4c 100644
>> --- a/configs/arm64/k3-j721e-evm-linux-demo.c
>> +++ b/configs/arm64/k3-j721e-evm-linux-demo.c
>> @@ -27,7 +27,7 @@ struct {
>> struct jailhouse_memory mem_regions[22];
>> struct jailhouse_irqchip irqchips[4];
>> struct jailhouse_pci_device pci_devices[2];
>> - __u32 stream_ids[2];
>> + union jailhouse_stream_id stream_ids[2];
>> } __attribute__((packed)) config = {
>> .cell = {
>> .signature = JAILHOUSE_CELL_DESC_SIGNATURE,
>> diff --git a/configs/arm64/k3-j721e-evm.c b/configs/arm64/k3-j721e-evm.c
>> index ab13fedd..aa5b47a9 100644
>> --- a/configs/arm64/k3-j721e-evm.c
>> +++ b/configs/arm64/k3-j721e-evm.c
>> @@ -22,7 +22,7 @@ struct {
>> struct jailhouse_memory mem_regions[40];
>> struct jailhouse_irqchip irqchips[6];
>> struct jailhouse_pci_device pci_devices[2];
>> - __u32 stream_ids[30];
>> + union jailhouse_stream_id stream_ids[30];
>> } __attribute__((packed)) config = {
>> .header = {
>> .signature = JAILHOUSE_SYSTEM_SIGNATURE,
>> diff --git a/configs/arm64/ultra96.c b/configs/arm64/ultra96.c
>> index db65ae01..19be84ae 100644
>> --- a/configs/arm64/ultra96.c
>> +++ b/configs/arm64/ultra96.c
>> @@ -21,7 +21,7 @@ struct {
>> struct jailhouse_memory mem_regions[11];
>> struct jailhouse_irqchip irqchips[1];
>> struct jailhouse_pci_device pci_devices[2];
>> - __u32 stream_ids[2];
>> + union jailhouse_stream_id stream_ids[2];
>> } __attribute__((packed)) config = {
>> .header = {
>> .signature = JAILHOUSE_SYSTEM_SIGNATURE,
>> @@ -161,6 +161,13 @@ struct {
>> },
>>
>> .stream_ids = {
>> - 0x870, 0x871
>> + {
>> + .mmu500.mask = 0x0,
>> + .mmu500.id = 0x870,
>> + },
>> + {
>> + .mmu500.mask = 0x0,
>> + .mmu500.id = 0x871,
>
> Only realizing now: That mask is an "ignore mask", right? Bits set there
> are NOT matched against the id. That's modeled after the hardware? Is
> this really intuitive? This one confused me at least.
Yes, it is model after the hardware. Maybe "ignore_mask" or "imask" would be
better?
Thanks,
Andrea
>
>> + },
>> },
>> };
>> diff --git a/configs/arm64/zynqmp-zcu102.c b/configs/arm64/zynqmp-zcu102.c
>> index bdcb04b3..1a5d29b3 100644
>> --- a/configs/arm64/zynqmp-zcu102.c
>> +++ b/configs/arm64/zynqmp-zcu102.c
>> @@ -23,7 +23,7 @@ struct {
>> struct jailhouse_memory mem_regions[12];
>> struct jailhouse_irqchip irqchips[1];
>> struct jailhouse_pci_device pci_devices[2];
>> - __u32 stream_ids[8];
>> + union jailhouse_stream_id stream_ids[3];
>> } __attribute__((packed)) config = {
>> .header = {
>> .signature = JAILHOUSE_SYSTEM_SIGNATURE,
>> @@ -147,6 +147,17 @@ struct {
>> },
>>
>> .stream_ids = {
>> - 0x860, 0x861, 0x870, 0x871, 0x874, 0x875, 0x876, 0x877
>> + {
>> + .mmu500.mask = 0x0,
>> + .mmu500.id = 0x860,
>> + },
>> + {
>> + .mmu500.mask = 0x0,
>> + .mmu500.id = 0x861,
>> + },
>> + {
>> + .mmu500.mask = 0xf,
>> + .mmu500.id = 0x870,
>> + },
>> },
>> };
>> diff --git a/hypervisor/arch/arm64/smmu-v3.c
>> b/hypervisor/arch/arm64/smmu-v3.c
>> index d4b7529d..d306818e 100644
>> --- a/hypervisor/arch/arm64/smmu-v3.c
>> +++ b/hypervisor/arch/arm64/smmu-v3.c
>> @@ -1045,8 +1045,9 @@ static int arm_smmuv3_cell_init(struct cell *cell)
>> struct arm_smmu_device *smmu = &smmu_devices[0];
>> struct jailhouse_iommu *iommu;
>> struct arm_smmu_cmdq_ent cmd;
>> - int ret, sid;
>> + int ret;
>> unsigned int n, s;
>> + union jailhouse_stream_id sid;
>
> I prefer (inverted) X-mas tree ordering, not only during this season.
>
>>
>> if (!iommu_count_units())
>> return 0;
>> @@ -1057,7 +1058,7 @@ static int arm_smmuv3_cell_init(struct cell *cell)
>> continue;
>>
>> for_each_stream_id(sid, cell->config, s) {
>> - ret = arm_smmu_init_ste(smmu, sid, cell->config->id);
>> + ret = arm_smmu_init_ste(smmu, sid.id, cell->config->id);
>> if (ret)
>> return ret;
>> }
>> @@ -1076,7 +1077,7 @@ static void arm_smmuv3_cell_exit(struct cell *cell)
>> struct arm_smmu_device *smmu = &smmu_devices[0];
>> struct jailhouse_iommu *iommu;
>> struct arm_smmu_cmdq_ent cmd;
>> - int sid;
>> + union jailhouse_stream_id sid;
>> unsigned int n, s;
>>
>> if (!iommu_count_units())
>> @@ -1088,7 +1089,7 @@ static void arm_smmuv3_cell_exit(struct cell *cell)
>> continue;
>>
>> for_each_stream_id(sid, cell->config, s) {
>> - arm_smmu_uninit_ste(smmu, sid, cell->config->id);
>> + arm_smmu_uninit_ste(smmu, sid.id, cell->config->id);
>> }
>>
>> cmd.opcode = CMDQ_OP_TLBI_S12_VMALL;
>> diff --git a/hypervisor/arch/arm64/smmu.c b/hypervisor/arch/arm64/smmu.c
>> index df92fb7a..9c625c54 100644
>> --- a/hypervisor/arch/arm64/smmu.c
>> +++ b/hypervisor/arch/arm64/smmu.c
>> @@ -84,6 +84,10 @@
>> #define SMR_VALID (1 << 31)
>> #define SMR_MASK_SHIFT 16
>> #define SMR_ID_SHIFT 0
>> +/* Ignore upper bit in ID and MASK */
>> +#define SMR_GET_ID(smr) ((smr) & 0x7fff)
>> +/* Mask is already specified from bit 0 in the configuration */
>> +#define SMR_GET_MASK(smr) ((smr) & 0x7fff)
>
> BIT_MASK(14, 0)
>
>>
>> /* Stream-to-Context Register */
>> #define ARM_SMMU_GR0_S2CR(n) (0xc00 + ((n) << 2))
>> @@ -152,7 +156,6 @@ struct arm_smmu_device {
>> unsigned long pgshift;
>> u32 num_context_banks;
>> u32 num_mapping_groups;
>> - u16 arm_sid_mask;
>> struct arm_smmu_smr *smrs;
>> };
>>
>> @@ -360,7 +363,7 @@ static int arm_smmu_device_cfg_probe(struct
>> arm_smmu_device *smmu)
>> return 0;
>> }
>>
>> -static int arm_smmu_find_sme(u16 id, struct arm_smmu_device *smmu)
>> +static int arm_smmu_find_sme(u16 id, u16 mask, struct arm_smmu_device *smmu)
>> {
>> struct arm_smmu_smr *smrs = smmu->smrs;
>> int free_idx = -EINVAL;
>> @@ -388,7 +391,7 @@ static int arm_smmu_find_sme(u16 id, struct
>> arm_smmu_device *smmu)
>> * expect simply identical entries for this case, but there's
>> * no harm in accommodating the generalisation.
>> */
>> - if ((smmu->arm_sid_mask & smrs[n].mask) == smmu->arm_sid_mask &&
>> + if ((mask & smrs[n].mask) == mask &&
>> !((id ^ smrs[n].id) & ~smrs[n].mask)) {
>> return n;
>> }
>> @@ -397,7 +400,7 @@ static int arm_smmu_find_sme(u16 id, struct
>> arm_smmu_device *smmu)
>> * though, then there always exists at least one stream ID
>> * which would cause a conflict, and we can't allow that risk.
>> */
>> - if (!((id ^ smrs[n].id) & ~(smrs[n].mask | smmu->arm_sid_mask)))
>> + if (!((id ^ smrs[n].id) & ~(smrs[n].mask | mask)))
>> return -EINVAL;
>> }
>>
>> @@ -409,7 +412,9 @@ static int arm_smmu_cell_init(struct cell *cell)
>> unsigned int vmid = cell->config->id;
>> struct arm_smmu_device *smmu;
>> struct arm_smmu_smr *smr;
>> - unsigned int dev, n, sid;
>> + unsigned int dev, n;
>> + u16 sid, smask;
>> + union jailhouse_stream_id fsid;
>> int ret, idx;
>>
>> /* If no sids, ignore */
>> @@ -421,19 +426,22 @@ static int arm_smmu_cell_init(struct cell *cell)
>>
>> smr = smmu->smrs;
>>
>> - for_each_stream_id(sid, cell->config, n) {
>> - ret = arm_smmu_find_sme(sid, smmu);
>> + for_each_stream_id(fsid, cell->config, n) {
>> + sid = SMR_GET_ID(fsid.mmu500.id);
>> + smask = SMR_GET_MASK(fsid.mmu500.mask);
>> +
>> + ret = arm_smmu_find_sme(sid, smask, smmu);
>> if (ret < 0)
>> return trace_error(ret);
>> idx = ret;
>>
>> - printk("Assigning StreamID 0x%x to cell \"%s\"\n",
>> - sid, cell->config->name);
>> + printk("Assigning SID 0x%x, Mask 0x%x to cell \"%s\"\n",
>> + sid, smask, cell->config->name);
>>
>> arm_smmu_write_s2cr(smmu, idx, S2CR_TYPE_TRANS, vmid);
>>
>> smr[idx].id = sid;
>> - smr[idx].mask = smmu->arm_sid_mask;
>> + smr[idx].mask = smask;
>> smr[idx].valid = true;
>>
>> arm_smmu_write_smr(smmu, idx);
>> @@ -449,14 +457,18 @@ static int arm_smmu_cell_init(struct cell *cell)
>> }
>>
>> static bool arm_smmu_return_sid_to_root_cell(struct arm_smmu_device *smmu,
>> - unsigned int sid, int idx)
>> + union jailhouse_stream_id fsid,
>> + int idx)
>> {
>> - unsigned int root_sid, n;
>> + unsigned int n;
>> + union jailhouse_stream_id rsid;
>>
>> - for_each_stream_id(root_sid, root_cell.config, n) {
>> - if (sid == root_sid) {
>> - printk("Assigning StreamID 0x%x to cell \"%s\"\n",
>> - sid, root_cell.config->name);
>> + for_each_stream_id(rsid, root_cell.config, n) {
>> + if (fsid.id == rsid.id) {
>> + printk("Assigning SID 0x%x Mask: 0x%x to cell \"%s\"\n",
>> + SMR_GET_ID(fsid.mmu500.id),
>> + SMR_GET_MASK(fsid.mmu500.mask),
>> + root_cell.config->name);
>>
>> /* We just need to update S2CR, SMR can stay as is. */
>> arm_smmu_write_s2cr(smmu, idx, S2CR_TYPE_TRANS,
>> @@ -471,7 +483,9 @@ static void arm_smmu_cell_exit(struct cell *cell)
>> {
>> int id = cell->config->id;
>> struct arm_smmu_device *smmu;
>> - unsigned int dev, n, sid;
>> + unsigned int dev, n;
>> + u16 sid, smask;
>> + union jailhouse_stream_id fsid;
>> int idx;
>>
>> /* If no sids, ignore */
>> @@ -479,10 +493,16 @@ static void arm_smmu_cell_exit(struct cell *cell)
>> return;
>>
>> for_each_smmu(smmu, dev) {
>> - for_each_stream_id(sid, cell->config, n) {
>> - idx = arm_smmu_find_sme(sid, smmu);
>> - if (idx < 0 ||
>> - arm_smmu_return_sid_to_root_cell(smmu, sid, idx))
>> + for_each_stream_id(fsid, cell->config, n) {
>> + sid = SMR_GET_ID(fsid.mmu500.id);
>> + smask = SMR_GET_MASK(fsid.mmu500.mask);
>> +
>> + idx = arm_smmu_find_sme(sid, smask, smmu);
>> + if (idx < 0)
>> + continue;
>> +
>> + /* return full stream ids */
>> + if (arm_smmu_return_sid_to_root_cell(smmu, fsid, idx))
>> continue;
>>
>> if (smmu->smrs) {
>> @@ -546,8 +566,6 @@ static int arm_smmu_init(void)
>> continue;
>>
>> smmu = &smmu_device[num_smmu_devices];
>> - smmu->arm_sid_mask = iommu->arm_mmu500.sid_mask;
>> -
>> smmu->base = paging_map_device(iommu->base, iommu->size);
>> if (!smmu->base) {
>> err = -ENOMEM;
>> diff --git a/hypervisor/arch/arm64/ti-pvu.c b/hypervisor/arch/arm64/ti-pvu.c
>> index 98c1fb5e..bbc633a0 100644
>> --- a/hypervisor/arch/arm64/ti-pvu.c
>> +++ b/hypervisor/arch/arm64/ti-pvu.c
>> @@ -444,7 +444,8 @@ int pvu_iommu_unmap_memory(struct cell *cell,
>>
>> void pvu_iommu_config_commit(struct cell *cell)
>> {
>> - unsigned int i, virtid;
>> + unsigned int i;
>> + union jailhouse_stream_id virtid;
>>
>> if (pvu_count == 0 || !cell)
>> return;
>> @@ -459,10 +460,10 @@ void pvu_iommu_config_commit(struct cell *cell)
>> cell->arch.iommu_pvu.ent_count);
>>
>> for_each_stream_id(virtid, cell->config, i) {
>> - if (virtid > MAX_VIRTID)
>> + if (virtid.id > MAX_VIRTID)
>> continue;
>>
>> - pvu_iommu_program_entries(cell, virtid);
>> + pvu_iommu_program_entries(cell, virtid.id);
>> }
>>
>> cell->arch.iommu_pvu.ent_count = 0;
>> @@ -470,8 +471,9 @@ void pvu_iommu_config_commit(struct cell *cell)
>>
>> static int pvu_iommu_cell_init(struct cell *cell)
>> {
>> - unsigned int i, virtid;
>> + unsigned int i;
>> struct pvu_dev *dev;
>> + union jailhouse_stream_id virtid;
>>
>> if (pvu_count == 0)
>> return 0;
>> @@ -484,10 +486,10 @@ static int pvu_iommu_cell_init(struct cell *cell)
>> dev = &pvu_units[0];
>> for_each_stream_id(virtid, cell->config, i) {
>>
>> - if (virtid > MAX_VIRTID)
>> + if (virtid.id > MAX_VIRTID)
>> continue;
>>
>> - if (pvu_tlb_is_enabled(dev, virtid))
>> + if (pvu_tlb_is_enabled(dev, virtid.id))
>> return -EINVAL;
>> }
>> return 0;
>> @@ -515,17 +517,18 @@ static int pvu_iommu_flush_context(u16 virtid)
>>
>> static void pvu_iommu_cell_exit(struct cell *cell)
>> {
>> - unsigned int i, virtid;
>> + unsigned int i;
>> + union jailhouse_stream_id virtid;
>>
>> if (pvu_count == 0)
>> return;
>>
>> for_each_stream_id(virtid, cell->config, i) {
>>
>> - if (virtid > MAX_VIRTID)
>> + if (virtid.id > MAX_VIRTID)
>> continue;
>>
>> - pvu_iommu_flush_context(virtid);
>> + pvu_iommu_flush_context(virtid.id);
>> }
>>
>> cell->arch.iommu_pvu.ent_count = 0;
>> diff --git a/include/jailhouse/cell-config.h
>> b/include/jailhouse/cell-config.h
>> index 472cb4bb..c94385b8 100644
>> --- a/include/jailhouse/cell-config.h
>> +++ b/include/jailhouse/cell-config.h
>> @@ -279,13 +279,18 @@ struct jailhouse_iommu {
>> __u64 tlb_base;
>> __u32 tlb_size;
>> } __attribute__((packed)) tipvu;
>> -
>> - struct {
>> - __u32 sid_mask;
>> - } __attribute__((packed)) arm_mmu500;
>> };
>> } __attribute__((packed));
>>
>> +union jailhouse_stream_id {
>> + __u32 id;
>> + struct {
>> + /* Note: both mask and id are only 15 bits wide */
>> + __u16 mask;
>> + __u16 id;
>> + } __attribute__((packed)) mmu500;
>> +} __attribute__((packed));
>> +
>> struct jailhouse_pio {
>> __u16 base;
>> __u16 length;
>> @@ -424,10 +429,11 @@ jailhouse_cell_pci_caps(const struct
>> jailhouse_cell_desc *cell)
>> cell->num_pci_devices * sizeof(struct jailhouse_pci_device));
>> }
>>
>> -static inline const __u32 *
>> +static inline const union jailhouse_stream_id *
>> jailhouse_cell_stream_ids(const struct jailhouse_cell_desc *cell)
>> {
>> - return (const __u32 *)((void *)jailhouse_cell_pci_caps(cell) +
>> + return (const union jailhouse_stream_id *)
>> + ((void *)jailhouse_cell_pci_caps(cell) +
>> cell->num_pci_caps * sizeof(struct jailhouse_pci_capability));
>> }
>>
>>
>
> Looks good to me otherwise.
>
> Jan
>
--
Thanks,
Andrea Bastoni
--
You received this message because you are subscribed to the Google Groups
"Jailhouse" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/jailhouse-dev/403b209e-8281-6f0b-409a-bd6dc14862c8%40tum.de.
