[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] x86/hvm: Improve hvm_set_guest_pat() code generation


  • To: Andrew Cooper <amc96@xxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Thu, 13 Jan 2022 15:59:47 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=f8ITnExiHu8U/qU56duDMiO+CW2rnT+AKznX+0j0i6Q=; b=T0PCX0n+eam78Fv9qmtTWj6teaJELdenOp296Umqy+2g4P6tcTYlXB+ShrgaE8OCmNn+qHZ5BrbIprRnZy9cNZIuf8pGcbzXxOAdi5pKTnuy9pZGTf/CT5/44MlXCPZkBtGhcq4OfxtD6esIl+HPJq2muDa4vYYlrUf2I81tBXg53TI8qnFp9ZM7LxBhg+VMOPrMn7+oueWN/wrEwZQly7Jl0Sar4JQ1TqvxE7v51x8l+n2plxBjAyUu1XvDEIy5TwLhNSqIVsNFRD6pNZQnnZc7rPXwDvaF+l226/5ZYggZmllr7DIRys69+2K+jsifrHOxuVWCmcTLUjLFrNN+nw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=UybwcT/5AnZh68q/5HPoe+C64MdYRgdNDYI6UNmKnIgYr5hp3bk5obT9DZIfjr664bKls94GUgB8W0Ga/p8Eh9eJHSFZ4LzQaJyFV51g76qoGMrFxJMe2m5f2rvLqlmX7ZbdZpJD5s4Ao8ghgYI3K2/BQvTi3jIi2/Z50+pN+hJWgTPlOs62sHErkgljnOU775KwVR78uJDCpgBhlplxVzJ0Eqz7I8wwPoKrw58Dei8UQ9O3yxzCjOimA529KT4HUNi29UK8xYY/2HdqT3LAzEYvIKUJz7Kgyn3trIKUr9wucFh8yj38VTa5moyK+UAaq5vVavXe6JuX2rBDSDU8/g==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Roger Pau Monné <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Thu, 13 Jan 2022 15:00:11 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 13.01.2022 15:45, Andrew Cooper wrote:
> On 13/01/2022 14:38, Jan Beulich wrote:
>> On 13.01.2022 14:50, Andrew Cooper wrote:
>>> This is a fastpath on virtual vmentry/exit, and forcing guest_pat to be
>>> spilled to the stack is bad.  Performing the shift in a register is far more
>>> efficient.
>>>
>>> Drop the (IMO useless) log message.  MSR_PAT only gets altered on boot, and 
>>> a
>>> bad value will be entirely evident in the ensuing #GP backtrace.
>>>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
>>
>> I'm curious though why ...
>>
>>> @@ -313,10 +313,9 @@ int hvm_set_guest_pat(struct vcpu *v, u64 guest_pat)
>>>          case PAT_TYPE_WRCOMB:
>>>          case PAT_TYPE_WRPROT:
>>>          case PAT_TYPE_WRTHROUGH:
>>> -            break;
>>> +            continue;
>> ... you're going from "break" to "continue" here.
> 
> I went through a couple of iterations, including one not having a switch
> statement at all.
> 
> Personally, I think continue is clearer to follow in constructs such as
> this, because it is clearly bound to the loop, while the break logic
> only works due to the switch being the final (only) clause.

Perhaps I was wrong recalling you somewhat disliking such uses of
"continue" in the past.

> P.S. if you want to see a hilarious Clang (mis)feature, check out
> https://godbolt.org/z/7z6PnKP31 - scroll to the bottom of the -O2 output.

"Nice".

Jan




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.