On 14.02.2022 16:51, George Dunlap wrote: > > >> On Jul 5, 2021, at 5:15 PM, Jan Beulich <jbeul...@suse.com> 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 <jbeul...@suse.com> > > Reviewed-by: George Dunlap <george.dun...@citrix.com>
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