On Fri, Jul 05, 2019 at 07:01:37PM +0800, Liu Yi L wrote:
> Intel VT-d 3.0 introduces scalable mode, and it has a bunch of
> capabilities related to scalable mode translation, thus there
> are multiple combinations. While this vIOMMU implementation
> wants simplify it for user by providing typical combinations.
> User could config it by "sm_model" option. The usage is as
> below:
>
> "-device intel-iommu,x-scalable-mode=on,sm_model=["legacy"|"scalable"]"
Is it a requirement to split into two parameters, instead of just
exposing everything about scalable mode when x-scalable-mode is set?
>
> - "legacy": gives support for SL page table
> - "scalable": gives support for FL page table, pasid, virtual command
> - default to be "legacy" if "x-scalable-mode=on while no sm_model is
> configured
>
> Cc: Kevin Tian <kevin.tian@xxxxxxxxx>
> Cc: Jacob Pan <jacob.jun.pan@xxxxxxxxxxxxxxx>
> Cc: Peter Xu <peterx@xxxxxxxxxx>
> Cc: Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx>
> Signed-off-by: Liu Yi L <yi.l.liu@xxxxxxxxx>
> Signed-off-by: Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx>
> ---
> hw/i386/intel_iommu.c | 28 +++++++++++++++++++++++++++-
> hw/i386/intel_iommu_internal.h | 2 ++
> include/hw/i386/intel_iommu.h | 1 +
> 3 files changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 44b1231..3160a05 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -3014,6 +3014,7 @@ static Property vtd_properties[] = {
> DEFINE_PROP_BOOL("caching-mode", IntelIOMMUState, caching_mode, FALSE),
> DEFINE_PROP_BOOL("x-scalable-mode", IntelIOMMUState, scalable_mode, FALSE),
> DEFINE_PROP_BOOL("dma-drain", IntelIOMMUState, dma_drain, true),
> + DEFINE_PROP_STRING("sm_model", IntelIOMMUState, sm_model),
Can do 's/-/_/' to follow the rest if we need it.
> DEFINE_PROP_END_OF_LIST(),
> };
>
> @@ -3489,6 +3490,14 @@ static void vtd_iommu_replay(IOMMUMemoryRegion *iommu_mr, IOMMUNotifier *n)
> return;
> }
>
> +const char sm_model_manual[] =
> + "\"-device intel-iommu,x-scalable-mode=on,"
> + "sm_model=[\"legacy\"|\"scalable\"]\"\n"
> + " - \"legacy\" gives support for SL page table based IOVA\n"
> + " - \"scalable\" gives support for FL page table based IOVA and SVA\n"
> + " - default to be \"legacy\" if \"x-scalable-mode=on\""
> + " while no sm_model is configured\n";
> +
> /* Do the initialization. It will also be called when reset, so pay
> * attention when adding new initialization stuff.
> */
> @@ -3557,9 +3566,26 @@ static void vtd_init(IntelIOMMUState *s)
> s->cap |= VTD_CAP_CM;
> }
>
> + if (s->sm_model && !s->scalable_mode) {
> + printf("\n\"sm_model\" depends on \"x-scalable-mode\"\n"
> + "please check if \"x-scalable-mode\" is expected\n"
> + "\"sm_model\" manual:\n%s", sm_model_manual);
> + exit(1);
Let's avoid calling exit() directly considering that we've had things
like vtd_decide_config() already which allows an Error**. We can also
introduce that too into vtd_init() and pass the error to upper to
handle the failure.
> + }
> +
> /* TODO: read cap/ecap from host to decide which cap to be exposed. */
> if (s->scalable_mode) {
> - s->ecap |= VTD_ECAP_SMTS | VTD_ECAP_SRS | VTD_ECAP_SLTS;
> + if (!s->sm_model || !strcmp(s->sm_model, "legacy")) {
> + s->ecap |= VTD_ECAP_SMTS | VTD_ECAP_SRS | VTD_ECAP_SLTS;
> + } else if (!strcmp(s->sm_model, "scalable")) {
> + s->ecap |= VTD_ECAP_SMTS | VTD_ECAP_SRS | VTD_ECAP_PASID
> + | VTD_ECAP_FLTS;
Do you also need VTD_ECAP_SLTS here?
> + } else {
> + printf("\n!!!!! Invalid sm_model config !!!!!\n"
> + "Please config sm_model=[\"legacy\"|\"scalable\"]\n"
> + "\"sm_model\" manual:\n%s", sm_model_manual);
> + exit(1);
Same here.
Thanks,
> + }
> }
>
> vtd_reset_caches(s);
> diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
> index c1235a7..adae198 100644
> --- a/hw/i386/intel_iommu_internal.h
> +++ b/hw/i386/intel_iommu_internal.h
> @@ -190,8 +190,10 @@
> #define VTD_ECAP_PT (1ULL << 6)
> #define VTD_ECAP_MHMV (15ULL << 20)
> #define VTD_ECAP_SRS (1ULL << 31)
> +#define VTD_ECAP_PASID (1ULL << 40)
> #define VTD_ECAP_SMTS (1ULL << 43)
> #define VTD_ECAP_SLTS (1ULL << 46)
> +#define VTD_ECAP_FLTS (1ULL << 47)
>
> /* CAP_REG */
> /* (offset >> 4) << 24 */
> diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
> index 12f3d26..b51cc9f 100644
> --- a/include/hw/i386/intel_iommu.h
> +++ b/include/hw/i386/intel_iommu.h
> @@ -270,6 +270,7 @@ struct IntelIOMMUState {
> bool buggy_eim; /* Force buggy EIM unless eim=off */
> uint8_t aw_bits; /* Host/IOVA address width (in bits) */
> bool dma_drain; /* Whether DMA r/w draining enabled */
> + char *sm_model; /* identify actual scalable mode iommu model*/
>
> /*
> * Protects IOMMU states in general. Currently it protects the
> --
> 2.7.4
>
Regards,
--
Peter Xu
