On 09/13/2018 04:46 PM, Wei Liu wrote:
> On Thu, Sep 06, 2018 at 05:20:53PM +0100, George Dunlap wrote:
>> On 09/04/2018 05:15 PM, Wei Liu wrote:
>>> These functions are only useful for nested hvm, which isn't enabled
>>> when CONFIG_HVM is false.
>>>
>>> Enclose relevant code and fields in CONFIG_HVM. Guard np2m_schedule
>>> with nestedhvm_enabled.
>>>
>>> Signed-off-by: Wei Liu <wei.l...@citrix.com>
>>> ---
>>> xen/arch/x86/domain.c | 6 ++++--
>>> xen/arch/x86/mm/p2m.c | 18 ++++++++++++++----
>>> xen/include/asm-x86/domain.h | 2 ++
>>> xen/include/asm-x86/p2m.h | 2 ++
>>> 4 files changed, 22 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
>>> index 313ebb3221..7c945a2428 100644
>>> --- a/xen/arch/x86/domain.c
>>> +++ b/xen/arch/x86/domain.c
>>> @@ -1691,7 +1691,8 @@ void context_switch(struct vcpu *prev, struct vcpu
>>> *next)
>>> {
>>> _update_runstate_area(prev);
>>> vpmu_switch_from(prev);
>>> - np2m_schedule(NP2M_SCHEDLE_OUT);
>>> + if ( nestedhvm_enabled(prevd) )
>>> + np2m_schedule(NP2M_SCHEDLE_OUT);
>>> }
>>>
>>> if ( is_hvm_domain(prevd) && !list_empty(&prev->arch.hvm.tm_list) )
>>> @@ -1758,7 +1759,8 @@ void context_switch(struct vcpu *prev, struct vcpu
>>> *next)
>>>
>>> /* Must be done with interrupts enabled */
>>> vpmu_switch_to(next);
>>> - np2m_schedule(NP2M_SCHEDLE_IN);
>>> + if ( nestedhvm_enabled(nextd) )
>>> + np2m_schedule(NP2M_SCHEDLE_IN);
>>
>> There's already a nestedhvm_enabled() check first thing in
>> np2m_schedule(). How does adding this check help the CONFIG_HVM cause?
>
> np2m_schedule will be gone entirely when !HVM. Add nestedhvm_enabled
> check here, which always evaluates to false when !HVM, makes compiler
> able to eliminate the call to np2m_schedule when linking.
>
>>
>> And why not #ifdef out this call, as well as the np2m_schedule()
>> functions entirely?
>
> The end result is the same. I'm happy to use either.
The end result may usually be the same in the case of !CONFIG_HVM, but
in the case of CONFIG_HVM, you'll have added a completely redundant
conditional. More importantly, if you use #ifdef, then it will be
immediately obvious to anyone reading the code that np2m_schedule isn't
called if !CONFIG_HVM.
Unless there are better reasons I'm not aware of, I'd prefer #ifdef here.
Thanks,
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
