From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH v2 05/12] VMX/altp2m: add code to support EPTP switching and #VE. Date: Wed, 24 Jun 2015 18:40:55 +0100 Message-ID: <558AEBA7.1090600@citrix.com> References: <1434999372-3688-1-git-send-email-edmund.h.white@intel.com> <1434999372-3688-6-git-send-email-edmund.h.white@intel.com> <558A9BBE.3050804@citrix.com> <558AE985.3000500@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <558AE985.3000500@intel.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ed White , xen-devel@lists.xen.org Cc: Ravi Sahita , Wei Liu , Tim Deegan , Ian Jackson , Jan Beulich , tlengyel@novetta.com, Daniel De Graaf List-Id: xen-devel@lists.xenproject.org On 24/06/15 18:31, Ed White wrote: > On 06/24/2015 04:59 AM, Andrew Cooper wrote: >>> + >>> + if ( !veinfo ) >>> + return 0; >>> + >>> + if ( veinfo->semaphore != 0 ) >>> + goto out; >> The semantics of this semaphore are not clearly spelled out in the >> manual. The only information I can locate concerning this field is in >> note in 25.5.6.1 which says: >> >> "Delivery of virtualization exceptions writes the value FFFFFFFFH to >> offset 4 in the virtualization-exception informa- >> tion area (see Section 25.5.6.2). Thus, once a virtualization exception >> occurs, another can occur only if software >> clears this field." >> >> I presume this should be taken to mean "software writes 0 to this >> field", but some clarification would be nice. >> > Immediately above that, where the conditions required to deliver #VE > are discussed, it says "the 32 bits at offset 4 in the > virtualization-exception information area are all 0". Hardware never > writes anything other than FFFFFFFFH there, so only software can make > that be so. So it does. Sorry for missing that. > >>> + >>> + if ( !p2m_find_altp2m_by_eptp(v->domain, eptp, &idx) ) >>> + { >>> + gdprintk(XENLOG_ERR, "EPTP not found in alternate p2m list\n"); >>> + domain_crash(v->domain); >>> + } >>> + } >>> + >> Is it worth checking that idx is plausible at this point, before blindly >> writing it back into the vcpu structure? > I'm not sure I follow your logic. In the case where the hardware supports > EPTP_INDEX, the hardware itself is asserting that the index is valid. In the > case quoted above, if the index isn't valid p2m_find_altp2m_by_eptp() will > fail. I tend to be somewhat more pessimistic when coding. It is possible for __vmread(EPTP_INDEX, &idx); to return any index between 0 and 511 (including some other software bug which has gone and accidentally set of altp2m 11). I would put a BUG_ON(idx >= MAX_ALTP2M) here just for safety. Nothing good can come of attempting to continue in such a state, but it is certainly not an impossible situation to get into. ~Andrew