[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v2 21/25] arm/altp2m: Add HVMOP_altp2m_change_gfn.
On 06/08/2016 19:42, Sergej Proskurin wrote:
Hi Julien,
Hello Sergej,
On 08/06/2016 04:34 PM, Julien Grall wrote:
On 06/08/2016 14:45, Sergej Proskurin wrote:
Hi Julien,
Hello Sergej,
On 08/04/2016 04:04 PM, Julien Grall wrote:
On 01/08/16 18:10, Sergej Proskurin wrote:
+ return rc;
+
+ hp2m = p2m_get_hostp2m(d);
+ ap2m = d->arch.altp2m_p2m[idx];
+
+ altp2m_lock(d);
+
+ /*
+ * Flip mem_access_enabled to true when a permission is set, as
to prevent
+ * allocating or inserting super-pages.
+ */
+ ap2m->mem_access_enabled = true;
Can you give more details about why you need this?
Similar to altp2m_set_mem_access, if we remap a page that is part of a
super page in the hostp2m, we first map the superpage in form of 512
pages into the ap2m and then change only one page. So, we set
mem_access_enabled to true to shatter the superpage on the ap2m side.
mem_access_enabled should only be set when mem access is enabled and
nothing.
I don't understand why you want to avoid superpage in the altp2m. If
you copy a host mapping is a superpage, then a altp2m mapping should
be a superpage.
The code is able to cope with inserting a mapping in the middle of a
superpage without mem_access_enabled.
Alright, I will try it out in the next patch. Thank you.
+
+ mfn = p2m_lookup_attr(ap2m, old_gfn, &p2mt, &level, NULL, NULL);
+
+ /* Check whether the page needs to be reset. */
+ if ( gfn_eq(new_gfn, INVALID_GFN) )
+ {
+ /* If mfn is mapped by old_gpa, remove old_gpa from the
altp2m table. */
+ if ( !mfn_eq(mfn, INVALID_MFN) )
+ {
+ rc = remove_altp2m_entry(d, ap2m, old_gpa,
pfn_to_paddr(mfn_x(mfn)), level);
remove_altp2m_entry should take a gfn and mfn in parameter and not an
address. The latter is a call for misusage of the API.
Ok. This will also remove the need for level_sizes/level_masks in the
associated function.
+ if ( rc )
+ {
+ rc = -EINVAL;
+ goto out;
+ }
+ }
+
+ rc = 0;
+ goto out;
+ }
+
+ /* Check host p2m if no valid entry in altp2m present. */
+ if ( mfn_eq(mfn, INVALID_MFN) )
+ {
+ mfn = p2m_lookup_attr(hp2m, old_gfn, &p2mt, &level, NULL,
&xma);
+ if ( mfn_eq(mfn, INVALID_MFN) || (p2mt != p2m_ram_rw) )
Please add a comment to explain why the second check.
Ok, I will. It has the same reason as in patch #19: It is not sufficient
so simply check for invalid MFN's as the type might be invalid. Also,
the x86 implementation did not allow to remap a gfn to a shared page.
Patch #19 has a different check which does not explain this one. (p2mt
!= p2m_ram_rw) only guest read-write RAM can effectively be remapped
which is different than shared page cannot be remapped.
BTW, ARM does not support shared page.
This also lead to my question, why not allowing p2m_ram_ro?
I don't see a reason why not. Thank you. I will remove the check.
Be careful, I never asked to remove the check. The p2m type contains
more than 2 cases, so if you remove completely the check it would be
possible to change device memory, grant, foreign mapping,...
The later will open a security issue because we require to have a
reference any foreign mapping before mapping it in the p2m.
So I think it only make sense to allow changing a gfn for p2m_ram_ro and
p2m_ram_rw.
+ {
+ rc = -EINVAL;
+ goto out;
+ }
+
+ /* If this is a superpage, copy that first. */
+ if ( level != 3 )
+ {
+ rc = modify_altp2m_entry(d, ap2m, old_gpa,
pfn_to_paddr(mfn_x(mfn)),
+ level, p2mt, memaccess[xma]);
+ if ( rc )
+ {
+ rc = -EINVAL;
+ goto out;
+ }
+ }
+ }
+
+ mfn = p2m_lookup_attr(ap2m, new_gfn, &p2mt, &level, NULL, &xma);
+
+ /* If new_gfn is not part of altp2m, get the mapping information
from hp2m */
+ if ( mfn_eq(mfn, INVALID_MFN) )
+ mfn = p2m_lookup_attr(hp2m, new_gfn, &p2mt, &level, NULL,
&xma);
+
+ if ( mfn_eq(mfn, INVALID_MFN) || (p2mt != p2m_ram_rw) )
Please add a comment to explain why the second check.
Same reason as above.
Then add a comment in the code.
I will also remove this check.
No. See my answer above.
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
