* [PATCH] x86/pv: Drop stale comment in dom0_construct_pv()
@ 2020-10-29 14:00 Andrew Cooper
2020-10-29 14:37 ` Jan Beulich
0 siblings, 1 reply; 3+ messages in thread
From: Andrew Cooper @ 2020-10-29 14:00 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich, Roger Pau Monné, Wei Liu
This comment has been around since c/s 1372bca0615 in 2004. It is stale, as
it predates the introduction of struct vcpu.
It is not obvious that it was even correct at the time. Where a vcpu (domain
at the time) has been configured to run is unrelated to construct the domain's
initial pagetables, etc.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
Almost... I'm not entirely sure NUMA memory allocation is plumbed through
correctly, but even that still has nothing to do with v->processor
---
xen/arch/x86/pv/dom0_build.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c
index d79503d6a9..f7165309a2 100644
--- a/xen/arch/x86/pv/dom0_build.c
+++ b/xen/arch/x86/pv/dom0_build.c
@@ -616,7 +616,6 @@ int __init dom0_construct_pv(struct domain *d,
v->arch.pv.event_callback_cs = FLAT_COMPAT_KERNEL_CS;
}
- /* WARNING: The new domain must have its 'processor' field filled in! */
if ( !is_pv_32bit_domain(d) )
{
maddr_to_page(mpt_alloc)->u.inuse.type_info = PGT_l4_page_table;
--
2.11.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] x86/pv: Drop stale comment in dom0_construct_pv()
2020-10-29 14:00 [PATCH] x86/pv: Drop stale comment in dom0_construct_pv() Andrew Cooper
@ 2020-10-29 14:37 ` Jan Beulich
2020-10-29 19:14 ` Andrew Cooper
0 siblings, 1 reply; 3+ messages in thread
From: Jan Beulich @ 2020-10-29 14:37 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Roger Pau Monné, Wei Liu, Xen-devel
On 29.10.2020 15:00, Andrew Cooper wrote:
> This comment has been around since c/s 1372bca0615 in 2004. It is stale, as
> it predates the introduction of struct vcpu.
That commit only moved it around; it's 22a857bde9b8 afaics from
early 2003 where it first appeared, where it had a reason:
/*
* WARNING: The new domain must have its 'processor' field
* filled in by now !!
*/
phys_l2tab = ALLOC_FRAME_FROM_DOMAIN();
l2tab = map_domain_mem(phys_l2tab);
memcpy(l2tab, idle_pg_table[p->processor], PAGE_SIZE);
But yes, the comment has been stale for a long time, and I've
been wondering a number of times what it was supposed to tell
me. (I think it was already stale at the point the comment
first got altered, in 3072fef54df8.)
> It is not obvious that it was even correct at the time. Where a vcpu (domain
> at the time) has been configured to run is unrelated to construct the domain's
> initial pagetables, etc.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Jan
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] x86/pv: Drop stale comment in dom0_construct_pv()
2020-10-29 14:37 ` Jan Beulich
@ 2020-10-29 19:14 ` Andrew Cooper
0 siblings, 0 replies; 3+ messages in thread
From: Andrew Cooper @ 2020-10-29 19:14 UTC (permalink / raw)
To: Jan Beulich; +Cc: Roger Pau Monné, Wei Liu, Xen-devel
On 29/10/2020 14:37, Jan Beulich wrote:
> On 29.10.2020 15:00, Andrew Cooper wrote:
>> This comment has been around since c/s 1372bca0615 in 2004. It is stale, as
>> it predates the introduction of struct vcpu.
> That commit only moved it around; it's 22a857bde9b8 afaics from
> early 2003 where it first appeared, where it had a reason:
>
> /*
> * WARNING: The new domain must have its 'processor' field
> * filled in by now !!
> */
> phys_l2tab = ALLOC_FRAME_FROM_DOMAIN();
> l2tab = map_domain_mem(phys_l2tab);
> memcpy(l2tab, idle_pg_table[p->processor], PAGE_SIZE);
Oh yes - my simple search didn't spot the reformat.
>
> But yes, the comment has been stale for a long time, and I've
> been wondering a number of times what it was supposed to tell
> me. (I think it was already stale at the point the comment
> first got altered, in 3072fef54df8.)
Looks like it became stale with 99db02d5097 "Remove CPU-dependent
page-directory entries." which drops the per-cpu idle_pg_table.
>
>> It is not obvious that it was even correct at the time. Where a vcpu (domain
>> at the time) has been configured to run is unrelated to construct the domain's
>> initial pagetables, etc.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>
Thanks. I'll update the commit message.
~Andrew
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-10-29 19:15 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-29 14:00 [PATCH] x86/pv: Drop stale comment in dom0_construct_pv() Andrew Cooper
2020-10-29 14:37 ` Jan Beulich
2020-10-29 19:14 ` Andrew Cooper
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.