> From: Peter Xu <peterx@xxxxxxxxxx> > Sent: Wednesday, November 6, 2019 10:01 PM > To: Liu, Yi L <yi.l.liu@xxxxxxxxx> > Subject: Re: [RFC v2 10/22] intel_iommu: add virtual command capability support > > On Wed, Nov 06, 2019 at 12:40:41PM +0000, Liu, Yi L wrote: > > > > > > Do you know what should happen on bare-metal from spec-wise that when > > > the guest e.g. writes 2 bytes to these mmio regions? > > > > I've no idea to your question. It is not a bare-metal capability. Personally, I > > prefer to have a toggle bit to mark the full written of a cmd to VMCD_REG. > > Reason is that we have no control on guest software. It may write new cmd > > to VCMD_REG in a bad manner. e.g. write high 32 bits first and then write the > > low 32 bits. Then it will have two traps. Apparently, for the first trap, it fills > > in the VCMD_REG and no need to handle it since it is not a full written. I'm > > checking it and evaluating it. How do you think on it? > > Oh I just noticed that vtd_mem_ops.min_access_size==4 now so writting > 2B should never happen at least. Then we'll bail out at > memory_region_access_valid(). Seems fine. got it. > > > > > > > > > > + if (!vtd_handle_vcmd_write(s, val)) { > > > > + vtd_set_long(s, addr, val); > > > > + } > > > > + break; > > > > + > > > > default: > > > > if (size == 4) { > > > > vtd_set_long(s, addr, val); > > > > @@ -3617,7 +3769,8 @@ static void vtd_init(IntelIOMMUState *s) > > > > s->ecap |= VTD_ECAP_SMTS | VTD_ECAP_SRS | VTD_ECAP_SLTS; > > > > } else if (!strcmp(s->scalable_mode, "modern")) { > > > > s->ecap |= VTD_ECAP_SMTS | VTD_ECAP_SRS | VTD_ECAP_PASID > > > > - | VTD_ECAP_FLTS | VTD_ECAP_PSS; > > > > + | VTD_ECAP_FLTS | VTD_ECAP_PSS | VTD_ECAP_VCS; > > > > + s->vccap |= VTD_VCCAP_PAS; > > > > } > > > > } > > > > > > > > > > [...] > > > > > > > +#define VTD_VCMD_CMD_MASK 0xffUL > > > > +#define VTD_VCMD_PASID_VALUE(val) (((val) >> 8) & 0xfffff) > > > > + > > > > +#define VTD_VCRSP_RSLT(val) ((val) << 8) > > > > +#define VTD_VCRSP_SC(val) (((val) & 0x3) << 1) > > > > + > > > > +#define VTD_VCMD_UNDEFINED_CMD 1ULL > > > > +#define VTD_VCMD_NO_AVAILABLE_PASID 2ULL > > > > > > According to 10.4.44 - should this be 1? > > > > It's 2 now per VT-d spec 3.1 (2019 June). I should have mentioned it in the cover > > letter... > > Well you're right... I hope there won't be other "major" things get > changed otherwise it'll be really a pain of working on all of these > before things settle... As far as I know, only this part has significant change. Other parts are consistent. I'll mention spec version next time in the cover letter. Thanks, Yi Liu