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

Re: [PATCH 16/16] x86/P2M: the majority for struct p2m_domain's fields are HVM-only


  • To: George Dunlap <George.Dunlap@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Wed, 16 Feb 2022 08:54:02 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; 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=7PA3hg//FA1dv8Cgam5+erljiIcWjJ8bYyyRvBR/Rxg=; b=h3qbhEb4u486ptxb8hMBYW+XfCp5M+nD0LL1t9hqm1TtaorKquMRv9nMuUp+omOd7w34jn356nyj6iD73ML9ZATbg0U0Gk8NuE5VVJ+qTFv77QJpov+8q0l7/qbMGPa05UcuvW3Vq5Wc761UCCLnEKSxxGay1rcVBjRjcrGZEDOMWyX+CxGd4yLM4XdvVsl0hF7PA71ah8ky+LgiIbL2yVOVb5qEYB9n/fknRBC3nP0iCZjFDd+JIqQ08Aze8rlwwc/kbfaoEuEmZEh575jXlWk/rdwp/aXsCDAAjMM6MpJNM/a5/IQd0EnljP9peFboxMRNCjlj5Gxs4GDB4v+5qg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ho97QhmDxwjjzk2fw7GTawOpYjdYwRpMtL4Ar7SPrwMGzcVsrfPYTnrSlr5WNb3KtFeWD0KK88wy/e5u4qraNVA9Xx21gqyQAcsHHkuJ2jta29jABRxocmL021bYJ0JEVUyk72Yg03Q7Idx6oqeUOMOa7L2/NwcavDoxfXODpmtc0t9S+/FzuKStoHUDhA4Spl4liA2rvPxsVqst+AR+Zxk6aD/n1Wc5oMiSiYBfnrPeHwgbe3eczMnQqK8jqXhMJO0/mCKEQ+Wmu5Ks3TU6EerWMEvcVpVAMgdL/zGuJfdBQJ9+84TpTowx/xe4uLngWDC/GvRfe7g9v2YJYph7Iw==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Roger Pau Monne <roger.pau@xxxxxxxxxx>, Paul Durrant <paul@xxxxxxx>
  • Delivery-date: Wed, 16 Feb 2022 07:54:20 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 14.02.2022 17:07, Jan Beulich wrote:
> On 14.02.2022 16:51, George Dunlap wrote:
>>> On Jul 5, 2021, at 5:15 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>
>>> ..., as are the majority of the locks involved. Conditionalize things
>>> accordingly.
>>>
>>> Also adjust the ioreq field's indentation at this occasion.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>
>> Reviewed-by: George Dunlap <george.dunlap@xxxxxxxxxx>
> 
> Thanks.
> 
>> With one question…
>>
>>> @@ -905,10 +917,10 @@ int p2m_altp2m_propagate_change(struct d
>>> /* Set a specific p2m view visibility */
>>> int p2m_set_altp2m_view_visibility(struct domain *d, unsigned int idx,
>>>                                    uint8_t visible);
>>> -#else
>>> +#else /* CONFIG_HVM */
>>> struct p2m_domain *p2m_get_altp2m(struct vcpu *v);
>>> static inline void p2m_altp2m_check(struct vcpu *v, uint16_t idx) {}
>>> -#endif
>>> +#endif /* CONFIG_HVM */
>>
>> This is relatively minor, but what’s the normal for how to label #else 
>> macros here?  Wouldn’t you normally see “#endif /* CONFIG_HVM */“ and think 
>> that the immediately preceding lines are compiled only if CONFIG_HVM is 
>> defined?  I.e., would this be more accurate to write “!CONFIG_HVM” here?
>>
>> I realize in this case it’s not a big deal since the #else is just three 
>> lines above it, but since you took the time to add the comment in there, it 
>> seems like it’s worth the time to have a quick think about whether that’s 
>> the right thing to do.
> 
> Hmm, yes, let me make this !CONFIG_HVM. I think we're not really
> consistent with this, but I agree it's more natural like you say.

Coming to write a similar construct elsewhere, I've realized this is
odd. Looking through include/asm/, the model generally used is

#ifdef CONFIG_xyz
#else /* !CONFIG_xyz */
#endif /* CONFIG_xyz */

That's what I'll switch to here then as well.

Jan




 


Rackspace

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