Re: [Xen-devel] [PATCH v6 7/9] xen/mem_access: Use __get_gfn_type_access in set_mem_access
On 29.01.2020 14:53, Tamas K Lengyel wrote:
> On Wed, Jan 29, 2020 at 6:10 AM Jan Beulich wrote:
>>
>> On 27.01.2020 19:06, Tamas K Lengyel wrote:
>>> Use __get_gfn_type_access instead of p2m->get_entry to trigger page-forking
>>> when the mem_access permission is being set on a page that has not yet been
>>> copied over from the parent.
>>
>> You talking of page-forking here, don't you mean ...
>>
>>> --- a/xen/arch/x86/mm/mem_access.c
>>> +++ b/xen/arch/x86/mm/mem_access.c
>>> @@ -303,11 +303,10 @@ static int set_mem_access(struct domain *d, struct
>>> p2m_domain *p2m,
>>> ASSERT(!ap2m);
>>> #endif
>>> {
>>> -mfn_t mfn;
>>> p2m_access_t _a;
>>> p2m_type_t t;
>>> -
>>> -mfn = p2m->get_entry(p2m, gfn, , &_a, 0, NULL, NULL);
>>> +mfn_t mfn = __get_gfn_type_access(p2m, gfn_x(gfn), , &_a,
>>> + P2M_ALLOC, NULL, false);
>>
>> ... P2M_UNSHARE here?
>
> No, P2M_UNSHARE is only required if you are doing a memory write.
> Setting memory access permissions is not a memory write, so it's
> sufficient to just allocate the p2m entry. P2M_ALLOCATE also
> encompasses forking the entry if there is a parent VM.
Ah, I see. And hence
Reviewed-by: Jan Beulich
>> Also shouldn't you have Cc-ed Petre and Alexandru on this patch
>> (for their R: entries) and at least George (perhaps also Andrew
>> and me) to get an ack, seeing that you're the only maintainer
>> of the file?
>
> I've ran ./add_maintainers.pl on the patches, not sure why noone else got
> CC-d.
It not picking up R: entries would seem like a bug to me. It not
knowing of nesting of maintainership is entirely expected (or
else it would also need to know that it's - in this case - you
who is invoking it).
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v6 7/9] xen/mem_access: Use __get_gfn_type_access in set_mem_access
On Wed, Jan 29, 2020 at 6:10 AM Jan Beulich wrote:
>
> On 27.01.2020 19:06, Tamas K Lengyel wrote:
> > Use __get_gfn_type_access instead of p2m->get_entry to trigger page-forking
> > when the mem_access permission is being set on a page that has not yet been
> > copied over from the parent.
>
> You talking of page-forking here, don't you mean ...
>
> > --- a/xen/arch/x86/mm/mem_access.c
> > +++ b/xen/arch/x86/mm/mem_access.c
> > @@ -303,11 +303,10 @@ static int set_mem_access(struct domain *d, struct
> > p2m_domain *p2m,
> > ASSERT(!ap2m);
> > #endif
> > {
> > -mfn_t mfn;
> > p2m_access_t _a;
> > p2m_type_t t;
> > -
> > -mfn = p2m->get_entry(p2m, gfn, , &_a, 0, NULL, NULL);
> > +mfn_t mfn = __get_gfn_type_access(p2m, gfn_x(gfn), , &_a,
> > + P2M_ALLOC, NULL, false);
>
> ... P2M_UNSHARE here?
No, P2M_UNSHARE is only required if you are doing a memory write.
Setting memory access permissions is not a memory write, so it's
sufficient to just allocate the p2m entry. P2M_ALLOCATE also
encompasses forking the entry if there is a parent VM.
>
> Also shouldn't you have Cc-ed Petre and Alexandru on this patch
> (for their R: entries) and at least George (perhaps also Andrew
> and me) to get an ack, seeing that you're the only maintainer
> of the file?
I've ran ./add_maintainers.pl on the patches, not sure why noone else got CC-d.
Tamas
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v6 7/9] xen/mem_access: Use __get_gfn_type_access in set_mem_access
On 27.01.2020 19:06, Tamas K Lengyel wrote:
> Use __get_gfn_type_access instead of p2m->get_entry to trigger page-forking
> when the mem_access permission is being set on a page that has not yet been
> copied over from the parent.
You talking of page-forking here, don't you mean ...
> --- a/xen/arch/x86/mm/mem_access.c
> +++ b/xen/arch/x86/mm/mem_access.c
> @@ -303,11 +303,10 @@ static int set_mem_access(struct domain *d, struct
> p2m_domain *p2m,
> ASSERT(!ap2m);
> #endif
> {
> -mfn_t mfn;
> p2m_access_t _a;
> p2m_type_t t;
> -
> -mfn = p2m->get_entry(p2m, gfn, , &_a, 0, NULL, NULL);
> +mfn_t mfn = __get_gfn_type_access(p2m, gfn_x(gfn), , &_a,
> + P2M_ALLOC, NULL, false);
... P2M_UNSHARE here?
Also shouldn't you have Cc-ed Petre and Alexandru on this patch
(for their R: entries) and at least George (perhaps also Andrew
and me) to get an ack, seeing that you're the only maintainer
of the file?
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
