On 19-02-12 14:46:29, Peter Xu wrote:
[...]
> > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> > index 3664a00..447fdf3 100644
> > --- a/hw/i386/intel_iommu.c
> > +++ b/hw/i386/intel_iommu.c
> > @@ -2492,6 +2492,17 @@ static bool vtd_process_inv_desc(IntelIOMMUState *s)
> > }
> > break;
> >
> > + /*
> > + * TODO: the entity of below two cases will be implemented in future
> > series.
> > + * To make guest (which integrates scalable mode support patch set in
> > + * iommu driver) work, just return true is enough so far.
>
> When you repost, could you mention about what tests have you done? I
> can think of some general usages:
>
> Device matrix:
>
> - virtio-net, vhost=on|off
> - device assignment
>
> Configuration matrix:
>
> - legacy/scalable
> - passthrough=on|off
>
> I believe smoke test (like netperf a few seconds, or even ping) would
> be enough, cover some (or all, which would be very nice to have :) of
> above scenarios.
>
Thanks for the test cases! I have covered some of them but I think I
can do more tests.
> > + */
> > + case VTD_INV_DESC_PC:
> > + break;
> > +
> > + case VTD_INV_DESC_PIOTLB:
> > + break;
> > +
> > case VTD_INV_DESC_WAIT:
> > trace_vtd_inv_desc("wait", inv_desc.val[1], inv_desc.val[0]);
> > if (!vtd_process_wait_desc(s, &inv_desc)) {
> > @@ -3051,6 +3062,7 @@ static Property vtd_properties[] = {
> > DEFINE_PROP_UINT8("aw-bits", IntelIOMMUState, aw_bits,
> > VTD_HOST_ADDRESS_WIDTH),
> > DEFINE_PROP_BOOL("caching-mode", IntelIOMMUState, caching_mode, FALSE),
> > + DEFINE_PROP_BOOL("scalable-mode", IntelIOMMUState, scalable_mode,
> > FALSE),
>
> Let's start with x-scalable-mode? Less burden for all.
>
Sure.
> > DEFINE_PROP_BOOL("dma-drain", IntelIOMMUState, dma_drain, true),
> > DEFINE_PROP_END_OF_LIST(),
> > };
> > @@ -3583,6 +3595,16 @@ static void vtd_init(IntelIOMMUState *s)
> > s->cap |= VTD_CAP_CM;
> > }
> >
> > + /* TODO: read cap/ecap from host to decide which cap to be exposed. */
> > + if (s->scalable_mode) {
> > + if (!s->caching_mode) {
> > + error_report("Need to set caching-mode for scalable mode");
>
> Could I ask why?
>
My intention is to make guest explicitly send to make sure SLT shadowing
correctly.
For this point, I also have question. Why does legacy mode not check CM?
If CM is not set, may the DMA remapping be wrong because SLT cannot
match guest's latest change?
> > + exit(1);
> > + }
> > + s->cap |= VTD_CAP_DWD | VTD_CAP_DRD;
>
> Draining is supported now so this line is not needed.
>
Got it, thanks!
> > + s->ecap |= VTD_ECAP_SMTS | VTD_ECAP_SRS | VTD_ECAP_SLTS;
> > + }
> > +
> > vtd_reset_caches(s);
> >
> > /* Define registers with default values and bit semantics */
> > diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
> > index 2a753c5..b01953a 100644
> > --- a/hw/i386/intel_iommu_internal.h
> > +++ b/hw/i386/intel_iommu_internal.h
> > @@ -190,7 +190,9 @@
> > #define VTD_ECAP_EIM (1ULL << 4)
> > #define VTD_ECAP_PT (1ULL << 6)
> > #define VTD_ECAP_MHMV (15ULL << 20)
> > +#define VTD_ECAP_SRS (1ULL << 31)
> > #define VTD_ECAP_SMTS (1ULL << 43)
> > +#define VTD_ECAP_SLTS (1ULL << 46)
> >
> > /* CAP_REG */
> > /* (offset >> 4) << 24 */
> > @@ -209,6 +211,8 @@
> > #define VTD_CAP_DRAIN_READ (1ULL << 55)
> > #define VTD_CAP_DRAIN (VTD_CAP_DRAIN_READ |
> > VTD_CAP_DRAIN_WRITE)
> > #define VTD_CAP_CM (1ULL << 7)
> > +#define VTD_CAP_DWD (1ULL << 54)
> > +#define VTD_CAP_DRD (1ULL << 55)
>
> These can be dropped too (see VTD_CAP_DRAIN above).
>
Thanks!
[...]
> > --
> > 1.9.1
> >
>
> Regards,
>
> --
> Peter Xu
