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 <[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.
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
