[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 08/16] x86/P2M: PoD, altp2m, and nested-p2m are HVM-only
- To: George Dunlap <George.Dunlap@xxxxxxxxxx>
- From: Jan Beulich <jbeulich@xxxxxxxx>
- Date: Mon, 7 Feb 2022 11:11:22 +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=8YH8xXPsjaApYBZN9h+yF+I+pqBSmrixHWlVUqXll7E=; b=QHx+86vzPIUGxrj9ShEde8FM13s6y/GGO23hnTN1luBkbLOJgnEDeM564nIxZ8jid2l2EeAmIRZDOkwQo2rHD6xgvFESp3/2LTcWKmrscRnqQzS4MBLgwFE68j1TLHHB3PxlEcm5W4aM4fk756cecv0Tc4AThg7ZXq7AlSTDb6dJ+YfxnBROVyGL6TKaCd/lZxPxh5eZ/Aip+C6RIuGRm+wz8I9FS1/HXJrV4RSO3qfkdbAX4NwfciQKholk0HoJ/6Yu7crodqlAu8UFyjTGac/+iaS5RKZRApwFe8aqbUyHvkLCvtxWqIUBUhRM5geMyusv5hfsUmQ3C6D199B4/A==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=i9/4kvfgOB2QWNmql3FuDFU1GVQeffmMYtfS8LMC4kOCm/NqlVZgpHKMzI3GqcQO7uHO5V+4gmWfVLh0BXDYRMqWF1KzEqcTStBpTQNEF9rE0cHozNywCuuKpU0xDwhfZCjEdO9VwquCQ5Htmwyrc5ts4FJqGyxafpx9IxXJJdv7xkuPPBxlzl2utjTERpkv0IU2tpatBS3rLAwHfaKuy9Q3gIt18l8wlVWAcagcKFGmX3G7JwQOsIPxzRTjmz8O2mo/U87jLcXoUB+qKwRn631gV5Ac2sUOmn0kGl5OP2lQa3clADO/USewmsfszx5GATOkRBr+t9HJMTvifTyqFw==
- 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>
- Delivery-date: Mon, 07 Feb 2022 10:11:36 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On 05.02.2022 22:29, George Dunlap wrote:
>> On Jul 5, 2021, at 5:09 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>> --- a/xen/arch/x86/mm/p2m-pod.c
>> +++ b/xen/arch/x86/mm/p2m-pod.c
>> @@ -1135,6 +1135,12 @@ p2m_pod_demand_populate(struct p2m_domai
>> mfn_t mfn;
>> unsigned long i;
>>
>> + if ( !p2m_is_hostp2m(p2m) )
>> + {
>> + ASSERT_UNREACHABLE();
>> + return false;
>> + }
>> +
>> ASSERT(gfn_locked_by_me(p2m, gfn));
>> pod_lock(p2m);
>
> Why this check rather than something which explicitly says HVM?
Checking for just HVM is too lax here imo. PoD operations should
never be invoked for alternative or nested p2ms; see the various
uses of p2m_get_hostp2m() in p2m-pod.c. However, looking at the
call sites again, I no longer see why I did put in
ASSERT_UNREACHABLE() here. IOW ...
> If you really mean to check for HVM here but are just using this as a
> shortcut, it needs a comment.
... it's not just a shortcut, yet it feels as if even then you'd
want a comment attached. I'm not really sure though what such a
comment might say which goes beyond what the use p2m_is_hostp2m()
already communicates.
> With that addressed:
>
> Reviewed-by: George Dunlap <george.dunlap@xxxxxxxxxx>
Thanks, but as per above I'll wait with making use of this.
Jan
|
