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?
Would it make sense to mark as tainted which could help identify the
issue on production builds? Maybe that's too much.
Thanks, Roger.
