On 14.02.2022 16:51, George Dunlap wrote:
>
>
>> On Jul 5, 2021, at 5:15 PM, Jan Beulich <[email protected]> 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 <[email protected]>
>
> Reviewed-by: George Dunlap <[email protected]>
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.
Jan
