From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: Yang Zhang <yang.z.zhang@intel.com>, Keir Fraser <keir@xen.org>,
Kevin Tian <kevin.tian@intel.com>, Feng Wu <feng.wu@intel.com>,
xen-devel@lists.xen.org
Subject: Re: [v4 11/17] vt-d: Add API to update IRTE when VT-d PI is used
Date: Thu, 23 Jul 2015 17:11:24 +0100 [thread overview]
Message-ID: <55B1122C.40107@citrix.com> (raw)
In-Reply-To: <55B12BC30200007800094C34@prv-mh.provo.novell.com>
On 23/07/15 17:00, Jan Beulich wrote:
>>>> On 23.07.15 at 17:55, <andrew.cooper3@citrix.com> wrote:
>> On 23/07/15 16:52, Jan Beulich wrote:
>>>>>> On 23.07.15 at 15:51, <andrew.cooper3@citrix.com> wrote:
>>>> On 23/07/15 12:35, Feng Wu wrote:
>>>>> + GET_IREMAP_ENTRY(ir_ctrl->iremap_maddr, remap_index, iremap_entries, p);
>>>>> +
>>>>> + old_ire = new_ire = *p;
>>>>> +
>>>>> + /* Setup/Update interrupt remapping table entry. */
>>>>> + setup_posted_irte(&new_ire, pi_desc, gvec);
>>>>> + ret = cmpxchg16b(p, &old_ire, &new_ire);
>>>>> +
>>>>> + ASSERT(ret == *(__uint128_t *)&old_ire);
>>>> This cannot be correct. Either the cmpxchg() is required and you must
>>>> cope with it failing, or the cmpxchg() is not required and this should
>>>> be a plain write.
>>> Not exactly: The cmpxchg() is required for this to be an atomic
>>> 128-bit write. And hence I would view the ASSERT() as
>>> appropriate - it validates that the entry didn't change behind our
>>> back.
>> But p is an active descriptor, which means hardware is liable to change
>> it behind our back.
> I inquired about this on the previous round and was told
> hardware doesn't alter the descriptor.
Ah - apologies for not noticing this.
> Comparing this with
> the spec, I couldn't spot any fields that I would suspect
> getting written. Which fields do you have in mind?
None in particular.
I called it out because ASSERT(cmpxchg(...) == old) reads as if it is buggy.
If hardware will genuinely never update the descriptor, then a comment
should be put in here explaining why the assert is safe in this instance.
~Andrew
next prev parent reply other threads:[~2015-07-23 16:11 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-23 11:35 [v4 00/17] Add VT-d Posted-Interrupts support Feng Wu
2015-07-23 11:35 ` [v4 01/17] VT-d Posted-intterrupt (PI) design Feng Wu
2015-07-23 11:35 ` [v4 02/17] Add helper macro for X86_FEATURE_CX16 feature detection Feng Wu
2015-07-23 11:35 ` [v4 03/17] Add cmpxchg16b support for x86-64 Feng Wu
2015-07-24 15:03 ` Jan Beulich
2015-07-23 11:35 ` [v4 04/17] iommu: Add iommu_intpost to control VT-d Posted-Interrupts feature Feng Wu
2015-07-23 14:01 ` Andrew Cooper
2015-07-23 14:05 ` Andrew Cooper
2015-07-24 0:47 ` Wu, Feng
2015-07-23 11:35 ` [v4 05/17] vt-d: VT-d Posted-Interrupts feature detection Feng Wu
2015-07-24 15:05 ` Jan Beulich
2015-07-23 11:35 ` [v4 06/17] vmx: Extend struct pi_desc to support VT-d Posted-Interrupts Feng Wu
2015-07-23 11:35 ` [v4 07/17] vmx: Add some helper functions for Posted-Interrupts Feng Wu
2015-07-23 11:35 ` [v4 08/17] vmx: Initialize VT-d Posted-Interrupts Descriptor Feng Wu
2015-07-23 11:35 ` [v4 09/17] vmx: Suppress posting interrupts when 'SN' is set Feng Wu
2015-07-24 15:11 ` Jan Beulich
2015-07-23 11:35 ` [v4 10/17] vt-d: Extend struct iremap_entry to support VT-d Posted-Interrupts Feng Wu
2015-07-24 15:13 ` Jan Beulich
2015-07-23 11:35 ` [v4 11/17] vt-d: Add API to update IRTE when VT-d PI is used Feng Wu
2015-07-23 13:51 ` Andrew Cooper
2015-07-23 15:52 ` Jan Beulich
2015-07-23 15:55 ` Andrew Cooper
2015-07-23 16:00 ` Jan Beulich
2015-07-23 16:11 ` Andrew Cooper [this message]
2015-07-24 0:39 ` Wu, Feng
2015-07-24 15:27 ` Jan Beulich
2015-07-28 7:34 ` Wu, Feng
2015-08-11 10:18 ` Jan Beulich
2015-07-23 11:35 ` [v4 12/17] Update IRTE according to guest interrupt config changes Feng Wu
2015-07-23 11:35 ` [v4 13/17] vmx: posted-interrupt handling when vCPU is blocked Feng Wu
2015-07-23 11:35 ` [v4 14/17] vmx: Properly handle notification event when vCPU is running Feng Wu
2015-07-23 11:35 ` [v4 15/17] arm: add a dummy arch hooks for scheduler Feng Wu
2015-07-23 11:54 ` Julien Grall
2015-07-24 0:39 ` Wu, Feng
2015-07-23 11:58 ` Jan Beulich
2015-07-23 11:35 ` [v4 16/17] vmx: Add some scheduler hooks for VT-d posted interrupts Feng Wu
2015-07-23 12:50 ` Dario Faggioli
2015-07-24 0:49 ` Wu, Feng
2015-07-28 14:15 ` Dario Faggioli
2015-07-30 2:04 ` Wu, Feng
2015-07-30 18:26 ` Dario Faggioli
2015-08-11 10:23 ` Jan Beulich
2015-07-23 11:35 ` [v4 17/17] VT-d: Dump the posted format IRTE Feng Wu
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=55B1122C.40107@citrix.com \
--to=andrew.cooper3@citrix.com \
--cc=JBeulich@suse.com \
--cc=feng.wu@intel.com \
--cc=keir@xen.org \
--cc=kevin.tian@intel.com \
--cc=xen-devel@lists.xen.org \
--cc=yang.z.zhang@intel.com \
/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).