From: "Jan Beulich" <JBeulich@suse.com>
To: Quan Xu <quan.xu@intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>,
Stefano Stabellini <sstabellini@kernel.org>,
Feng Wu <feng.wu@intel.com>,
dario.faggioli@citrix.com, xen-devel@lists.xen.org,
Julien Grall <julien.grall@arm.com>,
Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Subject: Re: [Patch v6 04/11] IOMMU: propagate IOMMU Device-TLB flush error up to IOMMU mapping (top level ones)
Date: Wed, 01 Jun 2016 04:24:01 -0600 [thread overview]
Message-ID: <574ED3E202000078000F0532@prv-mh.provo.novell.com> (raw)
In-Reply-To: <1464703056-4741-5-git-send-email-quan.xu@intel.com>
>>> On 31.05.16 at 15:57, <quan.xu@intel.com> wrote:
> --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
> +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
> @@ -295,12 +297,22 @@ static void __hwdom_init amd_iommu_hwdom_init(struct domain *d)
> * a pfn_valid() check would seem desirable here.
> */
> if ( mfn_valid(pfn) )
> - amd_iommu_map_page(d, pfn, pfn,
> - IOMMUF_readable|IOMMUF_writable);
> + {
> + int ret;
> +
> + ret = amd_iommu_map_page(d, pfn, pfn,
> + IOMMUF_readable|IOMMUF_writable);
Same here as for the earlier patch regarding assignment vs initializer.
But overall the entire change to this function seems to rather belong
into patch 2. As would a respective change to
vtd_set_hwdom_mapping(), which I'm not sure which patch you've
put that in.
> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -166,8 +166,8 @@ struct iommu_ops {
> #endif /* HAS_PCI */
>
> void (*teardown)(struct domain *d);
> - int (*map_page)(struct domain *d, unsigned long gfn, unsigned long mfn,
> - unsigned int flags);
> + int __must_check (*map_page)(struct domain *d, unsigned long gfn,
> + unsigned long mfn, unsigned int flags);
With this and with the rule set forth in the context of the discussion
of v5, iommu_map_page() (as well as any other caller of this hook
that do not themselves _consume_ the error [e.g. hwdom init ones])
should become or already be __must_check, which afaict isn't the
case. The same then, btw., applies to patch 3, and hence I have to
withdraw the R-b that you've got there.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
next prev parent reply other threads:[~2016-06-01 10:24 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-31 13:57 [Patch v6 00/11] Check VT-d Device-TLB flush error Xu, Quan
2016-05-31 13:57 ` [Patch v6 01/11] IOMMU: handle IOMMU mapping and unmapping failures Xu, Quan
2016-06-01 9:52 ` Jan Beulich
2016-05-31 13:57 ` [Patch v6 02/11] IOMMU/MMU: enhance the call trees of IOMMU unmapping and mapping Xu, Quan
2016-06-01 10:05 ` Jan Beulich
2016-06-02 6:00 ` Xu, Quan
2016-06-02 9:13 ` Jan Beulich
2016-05-31 13:57 ` [Patch v6 03/11] IOMMU: propagate IOMMU Device-TLB flush error up to IOMMU unmapping (top level ones) Xu, Quan
2016-05-31 13:57 ` [Patch v6 04/11] IOMMU: propagate IOMMU Device-TLB flush error up to IOMMU mapping " Xu, Quan
2016-06-01 10:24 ` Jan Beulich [this message]
2016-06-02 7:25 ` Xu, Quan
2016-06-02 9:21 ` Jan Beulich
2016-06-02 12:43 ` Xu, Quan
2016-06-07 7:51 ` Xu, Quan
2016-06-07 8:19 ` Jan Beulich
2016-06-07 8:40 ` Xu, Quan
2016-06-07 10:11 ` Jan Beulich
2016-05-31 13:57 ` [Patch v6 05/11] IOMMU/MMU: propagate IOMMU Device-TLB flush error up to iommu_iotlb_flush{, _all} " Xu, Quan
2016-05-31 13:57 ` [Patch v6 06/11] propagate IOMMU Device-TLB flush error up to EPT update " Xu, Quan
2016-05-31 13:57 ` [Patch v6 07/11] IOMMU: propagate IOMMU Device-TLB flush error up to IOMMU suspending " Xu, Quan
2016-06-01 10:39 ` Jan Beulich
2016-06-02 2:58 ` Xu, Quan
2016-06-02 9:22 ` Jan Beulich
2016-05-31 13:57 ` [Patch v6 08/11] IOMMU: propagate IOMMU Device-TLB flush error (leaf ones) Xu, Quan
2016-05-31 13:57 ` [Patch v6 09/11] vt-d: fix the IOMMU flush issue Xu, Quan
2016-06-01 15:36 ` Jan Beulich
2016-06-02 2:50 ` Xu, Quan
2016-05-31 13:57 ` [Patch v6 10/11] vt-d: propagate the IOMMU Device-TLB flush error up to ME phantom functions Xu, Quan
2016-05-31 13:57 ` [Patch v6 11/11] vt-d: add __must_check annotation to IOMMU flush pointers and handlers Xu, Quan
2016-06-02 10:06 ` Jan Beulich
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=574ED3E202000078000F0532@prv-mh.provo.novell.com \
--to=jbeulich@suse.com \
--cc=dario.faggioli@citrix.com \
--cc=feng.wu@intel.com \
--cc=julien.grall@arm.com \
--cc=kevin.tian@intel.com \
--cc=quan.xu@intel.com \
--cc=sstabellini@kernel.org \
--cc=suravee.suthikulpanit@amd.com \
--cc=xen-devel@lists.xen.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).