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

Re: [PATCH v2 3/3] amd/msr: implement VIRT_SPEC_CTRL for HVM guests using legacy SSBD


  • To: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Mon, 28 Mar 2022 17:32:13 +0200
  • 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=3bgbAq2eYta5h0QAy9d/DArmmTv6+XJryUq1+OxNot4=; b=e3d9gWZCLGzbJ5/8ZsTiQZfuvgTv8+0eBC/d0+vjAHHB2YZ1RS+4yYgCpLCoZSR1pDLdpJNLOqJhfgjeiUqmHW9gtQCcWXdlSQSmrsEW0CEudR2D4V+UFeOyFJGIDgVDLtSW/MqmwwL3hgN+eUV7Ixi16OWIn3FwsQ6Z7xmiHmn9L439EF/hZXAajFVHDlKr7HB/jQZPfXJTB5jl1gp1QqvV61rQSxQYgMnNdeDwvOhmzUG9bzJO7GQZm1PIO0a405lO76XBHq63fMXjdK7K3uV6E0COSvKxuBfn42qsqA+CKPg/nbCPX+2rZSeX8fJoRPDphJxzRtZEl8HhZ4ZFUQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=VpxQVp7EtDregEzM7qkTwH9OnQUooBhnCKJt0RvNdw9/jZqIneYBtyC+lhP2XY90XS9tNJ4O1TG+I+IwrMIg+OzilwHAI5+fKM4EEFkoMxQu4/LIxC8AyCP+0Mf3wKQpolp10OIsSkBkXQnrjOu9mnsVo6yu8p6DPmPNgktUX7S2wdEYkFp6DspC5QvlX/kBJIspU0uO0rMsU71Hj/fJOxWnIekjuxKaWTafxTOKmlFkNb+usqhg+Of5Q2iS+2bxC5C0h7QUdhCxnXe+S+KTF9zFOfME9/qhppj355DHncByUNOI2BvRW+IlVANZtMmVveRmNBVljDmC0aqxR8I3pA==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Mon, 28 Mar 2022 15:32:24 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 28.03.2022 17:24, Roger Pau Monné wrote:
> On Mon, Mar 28, 2022 at 04:21:02PM +0200, Jan Beulich wrote:
>> On 15.03.2022 15:18, Roger Pau Monne wrote:
>>> @@ -677,14 +680,17 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, 
>>> uint64_t val)
>>>          if ( !cp->extd.virt_ssbd )
>>>              goto gp_fault;
>>>  
>>> -        /*
>>> -         * Only supports SSBD bit, the rest are ignored. Only modify the 
>>> SSBD
>>> -         * bit in case other bits are set.
>>> -         */
>>> -        if ( val & SPEC_CTRL_SSBD )
>>> -            msrs->spec_ctrl.raw |= SPEC_CTRL_SSBD;
>>> +        /* Only supports SSBD bit, the rest are ignored. */
>>> +        if ( cpu_has_amd_ssbd )
>>> +        {
>>> +            /* Only modify the SSBD bit in case other bits are set. */
>>
>> While more a comment on the earlier patch introducing this wording, it
>> occurred to me only here that this is ambiguous: It can also be read as
>> "Only modify the SSBD bit as long as other bits are set."
> 
> Hm, no, that's not what I meant. I meant to note that here we are
> careful to only modify the SSBD bit of spec_ctrl, because other bits
> might be used for other purposes.

Right, I understand that's what you mean, and because I understand
the ambiguity also slipped my attention in the earlier patch.

> We can't do:
> 
> msrs->spec_ctrl.raw = SPEC_CTRL_SSBD;
> 
> But maybe this doesn't require a comment, as it seems to raise more
> questions than answer?

I wouldn't mind if (in the earlier patch) you simply dropped the 2nd
sentence. Or alternatively how about "Also only record the SSBD bit
to return for future reads" or something along these lines?

Jan




 


Rackspace

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