Re: [PATCH v2] VT-d: Don't assume register-based invalidation is always supported

[Julien Grall <julien@xxxxxxx>]

Hi,

It is not really my area of expertise but I wanted to jump on one 
comment below...
On 20/04/2021 12:38, Jan Beulich wrote:
> On 01.04.2020 22:06, Chao Gao wrote:
> > ---
> > Changes in v2:
> >   - verify system suspension and resumption with this patch applied
> >   - don't fall back to register-based interface if enabling qinval failed.
> >     see the change in init_vtd_hw().
> >   - remove unnecessary "queued_inval_supported" variable
> >   - constify the "struct vtd_iommu *" of has_register_based_invalidation()
> >   - coding-style changes
> ... while this suggests this is v2 of a recently sent patch, the
> submission is dated a little over a year ago. This is confusing.
> It is additionally confusing that there were two copies of it in
> my inbox, despite mails coming from a list normally getting
> de-duplicated somewhere at our end (I believe).
>
> > --- a/xen/drivers/passthrough/vtd/iommu.c
> > +++ b/xen/drivers/passthrough/vtd/iommu.c
> > @@ -1193,6 +1193,14 @@ int __init iommu_alloc(struct acpi_drhd_unit *drhd)
> >   
> >       iommu->cap = dmar_readq(iommu->reg, DMAR_CAP_REG);
> >       iommu->ecap = dmar_readq(iommu->reg, DMAR_ECAP_REG);
> > +    iommu->version = dmar_readl(iommu->reg, DMAR_VER_REG);
> > +
> > +    if ( !iommu_qinval && !has_register_based_invalidation(iommu) )
> > +    {
> > +        printk(XENLOG_WARNING VTDPREFIX "IOMMU %d: cannot disable Queued 
> > Invalidation.\n",
> > +               iommu->index);
> Here (and at least once more yet further down): We don't normally end
> log messages with a full stop. Easily addressable while committing, of
> course.
I can find a large number of cases where log messages are ended with a 
full stop... Actually it looks more natural to me than your suggestion.
Cheers,

--
Julien Grall