> From: Jan Beulich <jbeul...@suse.com>
> Sent: Wednesday, April 21, 2021 5:23 PM
>
> On 20.04.2021 18:17, Roger Pau Monné wrote:
> > On Tue, Apr 20, 2021 at 05:38:51PM +0200, Jan Beulich wrote:
> >> On 20.04.2021 17:08, Roger Pau Monné wrote:
> >>> On Thu, Apr 02, 2020 at 04:06:06AM +0800, Chao Gao wrote:
> >>>> --- a/xen/drivers/passthrough/vtd/qinval.c
> >>>> +++ b/xen/drivers/passthrough/vtd/qinval.c
> >>>> @@ -442,6 +442,23 @@ int enable_qinval(struct vtd_iommu *iommu)
> >>>> return 0;
> >>>> }
> >>>>
> >>>> +static int vtd_flush_context_noop(struct vtd_iommu *iommu, uint16_t
> did,
> >>>> + uint16_t source_id, uint8_t
> >>>> function_mask,
> >>>> + uint64_t type, bool
> >>>> flush_non_present_entry)
> >>>> +{
> >>>> + dprintk(XENLOG_ERR VTDPREFIX, "IOMMU: Cannot flush
> CONTEXT.\n");
> >>>> + return -EIO;
> >>>> +}
> >>>> +
> >>>> +static int vtd_flush_iotlb_noop(struct vtd_iommu *iommu, uint16_t
> did,
> >>>> + uint64_t addr, unsigned int size_order,
> >>>> + uint64_t type, bool
> >>>> flush_non_present_entry,
> >>>> + bool flush_dev_iotlb)
> >>>> +{
> >>>> + dprintk(XENLOG_ERR VTDPREFIX, "IOMMU: Cannot flush IOTLB.\n");
> >>>> + return -EIO;
> >>>> +}
> >>>
> >>> I think I would add an ASSERT_UNREACHABLE() to both noop handlers
> >>> above, as I would expect trying to use them without the proper mode
> >>> being configured would point to an error elsewhere?
> >>
> >> If such an assertion triggered e.g. during S3 suspend/resume, it may
> >> lead to the box simply not doing anything useful, without there being
> >> any way to know what went wrong. If instead the system at least
> >> managed to resume, the log message could be observed.
> >
> > Oh, OK then. I'm simply worried that people might ignore such one line
> > messages, maybe add a WARN?
>
> Hmm, yes, perhaps - would allow seeing right away where the call
> came from. Chao, I'd again be fine to flip the dprintk()-s to
> WARN()-s while committing. But of course only provided you and
> Kevin (as the maintainer) agree.
>
Looks good.
Reviewed-by: Kevin Tian <kevin.t...@intel.com>
