Re: [Xen-devel] [PATCH v2 08/25] arm/altp2m: Add HVMOP_altp2m_set_domain_state.

[Julien Grall <julien.grall@xxxxxxx>]

Hello Sergej,

On 06/08/2016 10:36, Sergej Proskurin wrote:
> (I did not finish answering all questions in the previous mail)
> On 08/03/2016 08:41 PM, Julien Grall wrote:
> > On 01/08/16 18:10, Sergej Proskurin wrote:
[...]

> > > +
> > > +    /* Initialize the new altp2m view. */
> > > +    rc = p2m_init_one(d, p2m);
> > > +    if ( rc )
> > > +        goto err;
> > > +
> > > +    /* Allocate a root table for the altp2m view. */
> > > +    rc = p2m_alloc_table(p2m);
> > > +    if ( rc )
> > > +        goto err;
> > > +
> > > +    p2m->p2m_class = p2m_alternate;
> > > +    p2m->access_required = 1;
> > Please use true here. Although, I am not sure why you want to enable
> > the access by default.
> Will do.
>
> p2m->access_required is true by default in the x86 implementation. Also,
> there is currently no way to manually set access_required on altp2m.
> Besides, I do not see a scenario, where it makes sense to run altp2m
> without access_required set to true.
I am afraid to say that "x86 does it" is not an argument. When I am 
reading an ARM series I don't necessary look at the x86, which by way 
does not give any explanation which it set to true by default.
Please document it in the code.

Regards,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel