From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH RFC 26/31] xen/x86: Rework AMD masking MSR setup Date: Fri, 22 Jan 2016 17:03:12 +0000 Message-ID: <56A260D0.1080804@citrix.com> References: <1450301073-28191-1-git-send-email-andrew.cooper3@citrix.com> <1450301073-28191-27-git-send-email-andrew.cooper3@citrix.com> <56A2041702000078000C9EAB@prv-mh.provo.novell.com> <56A20C01.3010002@citrix.com> <56A21CD602000078000CA018@prv-mh.provo.novell.com> <56A235C2.3050600@citrix.com> <56A246CB02000078000CA1DD@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <56A246CB02000078000CA1DD@prv-mh.provo.novell.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: Jan Beulich Cc: Xen-devel List-Id: xen-devel@lists.xenproject.org On 22/01/16 14:12, Jan Beulich wrote: > >>>>> And then, how is this supposed to work? You only restore defaults, >>>>> but never write non-default values. Namely, nextd is an unused >>>>> function parameter ... >>>>> >>>>> Also I guess my comment about adding unused code needs >>>>> repeating here. >>>> Future patches build on this, both using the parameter, and not using >>>> the defaults. >>>> >>>> I am also certain that if I did two patches, the first putting in a void >>>> function, and the second changing it to a pointer, your review would ask >>>> me to turn it into this. >>> Well, I realize things will all make sense by the end of the series, but >>> honestly in some of the cases I'm not sure the split between patches >>> was well done. >> If you can suggest a better ordering then I am all ears. > For example, move all the context switch logic into the patch > actually invoking the new hook. That still leaves more than > enough in the AMD and Intel rework patches. But the context switch logic is used by this patch, which is why it is introduced here. It takes the BSP/AP from the boot state into the default levelled state, by passing NULL as the pointer. See the final hunk, which modifies early_init_amd(). ~Andrew