Messages in this thread | | | Date | Tue, 5 Jun 2018 10:32:53 -0700 | From | Jacob Pan <> | Subject | Re: [PATCH v4 04/22] iommu/vt-d: add bind_pasid_table function |
| |
On Thu, 31 May 2018 10:09:46 +0100 Jean-Philippe Brucker <jean-philippe.brucker@arm.com> wrote:
> On 30/05/18 20:52, Jacob Pan wrote: > >> However I think the model number should be added to > >> pasid_table_config. For one thing it gives us a simple > >> sanity-check, but it also tells which other fields are valid in > >> pasid_table_config. Arm-smmu-v3 needs at least two additional > >> 8-bit fields describing the PASID table format (number of levels > >> and PASID0 behaviour), which are written to device context tables > >> when installing the PASID table pointer. > >> > > We had model number field in v2 of this patchset. My thought was > > that since the config info is meant to be generic, we shouldn't > > include model info. But I also think a simple sanity check can be > > useful, would that be sufficient to address Alex's concern? Of > > course we still need sysfs for more specific IOMMU features. > > > > Would this work? > > enum pasid_table_model { > > PASID_TABLE_FORMAT_HOST, > > PASID_TABLE_FORMAT_ARM_1LVL, > > PASID_TABLE_FORMAT_ARM_2LVL, > > I'd rather use a single PASID_TABLE_FORMAT_ARM, because "2LVL" may be > further split into 2LVL_4k or 2LVL_64k leaf tables... I think it's > best if I add an arch-specific field in pasid_table_config for that, > and for the PASID0 configuration, when adding FORMAT_ARM in a future > patch > sounds good. will only use PASID_TABLE_FORMAT_ARM. > > PASID_TABLE_FORMAT_AMD, > > PASID_TABLE_FORMAT_INTEL, > > }; > > > > /** > > * PASID table data used to bind guest PASID table to the host > > IOMMU. This will > > * enable guest managed first level page tables. > > * @version: for future extensions and identification of the data > > format > > * @bytes: size of this structure > > * @model: PASID table format for different IOMMU models > > * @base_ptr: PASID table pointer > > * @pasid_bits: number of bits supported in the guest PASID > > table, must be less > > * or equal than the host supported PASID size. > > */ > > struct pasid_table_config { > > __u32 version; > > #define PASID_TABLE_CFG_VERSION_1 1 > > __u32 bytes; > > "bytes" could be passed by VFIO as argument to bind_pasid_table, since > it can deduce it from argsz > Are you suggesting we wrap this struct in a vfio struct with argsz? or we directly use this struct?
I need to clarify how vfio will use this. - User program: struct pasid_table_config ptc = { .bytes = sizeof(ptc) }; ptc.version = 1; ioctl(device, VFIO_DEVICE_BIND_PASID_TABLE, &ptc);
- Kernel: minsz = offsetofend(struct pasid_table_config, pasid_bits); if (ptc.bytes < minsz) return -EINVAL;
| |