| Message ID | 20191223140409.32449-3-aisaila@bitdefender.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
On 12/23/19 2:04 PM, Alexandru Stefan ISAILA wrote: Why? -George
On 24.12.2019 10:01, George Dunlap wrote: > On 12/23/19 2:04 PM, Alexandru Stefan ISAILA wrote: > > Why? > This was a request from Jan. Alex
On 12/24/19 10:08 AM, Alexandru Stefan ISAILA wrote: > > > On 24.12.2019 10:01, George Dunlap wrote: >> On 12/23/19 2:04 PM, Alexandru Stefan ISAILA wrote: >> >> Why? >> > > This was a request from Jan. Yes, I saw the Requested-by. It still needs an explanation. -George
On 24.12.2019 12:15, George Dunlap wrote: > On 12/24/19 10:08 AM, Alexandru Stefan ISAILA wrote: >> >> >> On 24.12.2019 10:01, George Dunlap wrote: >>> On 12/23/19 2:04 PM, Alexandru Stefan ISAILA wrote: >>> >>> Why? >>> >> >> This was a request from Jan. > > Yes, I saw the Requested-by. It still needs an explanation. > This is what Jan said in V2: "All of this is not EPT-specific. Before adding more infrastructure to cover for this (here: another function parameter), how about moving these parts into vendor-independent code?" If there is a need for further explanation maybe Jan can help here. Alex
On 06.01.2020 12:55, Alexandru Stefan ISAILA wrote: > On 24.12.2019 12:15, George Dunlap wrote: >> On 12/24/19 10:08 AM, Alexandru Stefan ISAILA wrote: >>> >>> >>> On 24.12.2019 10:01, George Dunlap wrote: >>>> On 12/23/19 2:04 PM, Alexandru Stefan ISAILA wrote: >>>> >>>> Why? >>>> >>> >>> This was a request from Jan. >> >> Yes, I saw the Requested-by. It still needs an explanation. >> > > This is what Jan said in V2: > > "All of this is not EPT-specific. Before adding more infrastructure to > cover for this (here: another function parameter), how about moving > these parts into vendor-independent code?" > > If there is a need for further explanation maybe Jan can help here. No matter who it was that asked for something, there's no reason to have a completely empty commit message, unless the title is both change description and rationale at once. Perhaps a slightly re-written sentence like "Some of what this EPT-specific function does it not EPT-specific" would already do. You could go further and also state there what I've said in the second sentence. Jan
On 1/6/20 11:55 AM, Alexandru Stefan ISAILA wrote: > On 24.12.2019 12:15, George Dunlap wrote: >> On 12/24/19 10:08 AM, Alexandru Stefan ISAILA wrote: >>> >>> >>> On 24.12.2019 10:01, George Dunlap wrote: >>>> On 12/23/19 2:04 PM, Alexandru Stefan ISAILA wrote: >>>> >>>> Why? >>>> >>> >>> This was a request from Jan. >> >> Yes, I saw the Requested-by. It still needs an explanation. >> > > This is what Jan said in V2: > > "All of this is not EPT-specific. Before adding more infrastructure to > cover for this (here: another function parameter), how about moving > these parts into vendor-independent code?" > > If there is a need for further explanation maybe Jan can help here. Well "EPT-specific" and "vendor-independent" are enough to indicate the reason for the motion, but the title doesn't say either of those two things, and so the reader is left to guess. A title / message like this would have been fine: --- x86/mm: Pull non-EPT-specific altp2m handling code into vendor-independent code No functional changes. --- Or since that's a bit long, maybe: --- x86/mm: Pull vendor-independent altp2m code out of p2m-ept.c ...and into p2m.c. No functional changes. --- -George
On 1/6/20 11:55 AM, Alexandru Stefan ISAILA wrote: > On 24.12.2019 12:15, George Dunlap wrote: >> On 12/24/19 10:08 AM, Alexandru Stefan ISAILA wrote: >>> >>> >>> On 24.12.2019 10:01, George Dunlap wrote: >>>> On 12/23/19 2:04 PM, Alexandru Stefan ISAILA wrote: >>>> >>>> Why? >>>> >>> >>> This was a request from Jan. >> >> Yes, I saw the Requested-by. It still needs an explanation. >> > > This is what Jan said in V2: > > "All of this is not EPT-specific. Before adding more infrastructure to > cover for this (here: another function parameter), how about moving > these parts into vendor-independent code?" > > If there is a need for further explanation maybe Jan can help here. You don't have to make every clean-up patch that reviewers ask for; but if you do post a clean-up patch, it's your responsibility to make sure it's got a suitable description. The audience is not only the reviewer who asked for the patch, but also normal developers 5 years from now (perhaps yourself) who are trying to figure out why the change was made. -George
diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c index b5517769c9..d861cd7b51 100644 --- a/xen/arch/x86/mm/p2m-ept.c +++ b/xen/arch/x86/mm/p2m-ept.c @@ -1357,13 +1357,7 @@ void p2m_init_altp2m_ept(struct domain *d, unsigned int i) struct p2m_domain *hostp2m = p2m_get_hostp2m(d); struct ept_data *ept; - p2m->default_access = hostp2m->default_access; - p2m->domain = hostp2m->domain; - - p2m->global_logdirty = hostp2m->global_logdirty; p2m->ept.ad = hostp2m->ept.ad; - p2m->min_remapped_gfn = gfn_x(INVALID_GFN); - p2m->max_mapped_pfn = p2m->max_remapped_gfn = 0; ept = &p2m->ept; ept->mfn = pagetable_get_pfn(p2m_get_pagetable(p2m)); d->arch.altp2m_eptp[i] = ept->eptp; diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c index de832dcc6d..5b99d1eb97 100644 --- a/xen/arch/x86/mm/p2m.c +++ b/xen/arch/x86/mm/p2m.c @@ -2562,6 +2562,12 @@ static int p2m_activate_altp2m(struct domain *d, unsigned int idx) goto out; } + p2m->default_access = hostp2m->default_access; + p2m->domain = hostp2m->domain; + p2m->global_logdirty = hostp2m->global_logdirty; + p2m->min_remapped_gfn = gfn_x(INVALID_GFN); + p2m->max_mapped_pfn = p2m->max_remapped_gfn = 0; + p2m_init_altp2m_ept(d, idx); out: