>From: Jan Beulich [mailto:jbeul...@suse.com]
>Sent: Thursday, July 23, 2015 3:06 AM
>
>>>> On 23.07.15 at 01:01, <edmund.h.wh...@intel.com> wrote:
>> Add the remaining routines required to support enabling the alternate
>> p2m functionality.
>>
>> Signed-off-by: Ed White <edmund.h.wh...@intel.com>
>>
>> Reviewed-by: Andrew Cooper <andrew.coop...@citrix.com>
>> ---
>> Changes since v6:
>> rename altp2m lazy copier, make bool_t, use __put_gfn throughout,
>> and move to p2m.c, eliminating altp2m_hap.c
>> change various p2m_altp2m... routines from long to int
>> change uint16_t's/uint8_t's to unsigned int's
>> optimize remapped gfn tracking and altp2m invalidation check
>> mechanical changes due to patch 5 changes
>
>Taken together clearly requiring dropping any earlier review tags.
>
>Non-mm parts
>Acked-by: Jan Beulich <jbeul...@suse.com>
>
Ok - other maintainers, please review the mm parts - thanks.
>> +void p2m_flush_altp2m(struct domain *d) {
>> + unsigned int i;
>> +
>> + altp2m_list_lock(d);
>> +
>> + for ( i = 0; i < MAX_ALTP2M; i++ )
>> + {
>> + p2m_flush_table(d->arch.altp2m_p2m[i]);
>> + /* Uninit and reinit ept to force TLB shootdown */
>> + ept_p2m_uninit(d->arch.altp2m_p2m[i]);
>> + ept_p2m_init(d->arch.altp2m_p2m[i]);
>> + d->arch.altp2m_eptp[i] = INVALID_MFN;
>> + }
>
>And more EPT-specific code in a generic file...
>
This was mentioned explicitly as pending in the cover letter and in the patch
header.
" Patch 10: ... not done - abstracting some ept functionality from p2m"
>> +int p2m_destroy_altp2m_by_id(struct domain *d, unsigned int idx) {
>> + struct p2m_domain *p2m;
>> + int rc = -EINVAL;
>> +
>> + if ( !idx || idx > MAX_ALTP2M )
>
> >= (and then also elsewhere further down)?
>
Right.
>> + return rc;
>> +
>> + domain_pause_except_self(d);
>> +
>> + altp2m_list_lock(d);
>> +
>> + if ( d->arch.altp2m_eptp[idx] != INVALID_MFN )
>> + {
>> + p2m = d->arch.altp2m_p2m[idx];
>> +
>> + if ( !_atomic_read(p2m->active_vcpus) )
>> + {
>> + p2m_flush_table(d->arch.altp2m_p2m[idx]);
>> + /* Uninit and reinit ept to force TLB shootdown */
>> + ept_p2m_uninit(d->arch.altp2m_p2m[idx]);
>> + ept_p2m_init(d->arch.altp2m_p2m[idx]);
>> + d->arch.altp2m_eptp[idx] = INVALID_MFN;
>
>Perhaps worth making all of the above if() body a helper function (considering
>that the loop body in p2m_flush_altp2m() does exactly the same)?
>
Ok can consider.
>> @@ -758,6 +765,38 @@ bool_t p2m_switch_vcpu_altp2m_by_id(struct
>vcpu
>> *v, unsigned int idx);
>> /* Check to see if vcpu should be switched to a different p2m. */
>> void p2m_altp2m_check(struct vcpu *v, uint16_t idx);
>
>The context here suggests that I overlooked a still not replaced uint16_t.
>
Ok.
Just wanted to make sure these are also ok to do post 4.6
Thanks,
Ravi
>> +/* Flush all the alternate p2m's for a domain */ void
>> +p2m_flush_altp2m(struct domain *d);
>> +
>> +/* Alternate p2m paging */
>> +bool_t p2m_altp2m_lazy_copy(struct vcpu *v, paddr_t gpa,
>> + unsigned long gla, struct npfec npfec, struct p2m_domain **ap2m);
>> +
>> +/* Make a specific alternate p2m valid */ int
>> +p2m_init_altp2m_by_id(struct domain *d, unsigned int idx);
>> +
>> +/* Find an available alternate p2m and make it valid */ int
>> +p2m_init_next_altp2m(struct domain *d, uint16_t *idx);
>
>For this one, however, things really depend on the intended call sites, i.e. I
>could see reasons for this to be kept as is.
>
>Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
