>>> On 30.01.18 at 18:12, <[email protected]> wrote: > On 30/01/18 16:40, Jan Beulich wrote: >>>>> On 22.01.18 at 13:32, <[email protected]> wrote: >>> +static int pv_vcpu_init_xpti(struct vcpu *v) >>> +{ >>> + struct domain *d = v->domain; >>> + struct page_info *pg; >>> + void *ptr; >>> + struct cpu_info *info; >>> + unsigned long stack_bottom; >>> + int rc; >>> + >>> + /* Populate page tables. */ >>> + rc = create_perdomain_mapping(d, XPTI_START(v), STACK_PAGES, >>> + NIL(l1_pgentry_t *), NULL); >>> + if ( rc ) >>> + goto done; >>> + >>> + /* Map stacks. */ >>> + rc = create_perdomain_mapping(d, XPTI_START(v), IST_MAX, >>> + NULL, NIL(struct page_info *)); >>> + if ( rc ) >>> + goto done; >>> + >>> + ptr = alloc_xenheap_page(); >>> + if ( !ptr ) >>> + { >>> + rc = -ENOMEM; >>> + goto done; >>> + } >>> + clear_page(ptr); >>> + addmfn_to_perdomain_mapping(d, XPTI_START(v) + STACK_SIZE - PAGE_SIZE, >>> + _mfn(virt_to_mfn(ptr))); >> >> This can't be create_perdomain_mapping() because of ...? If it's >> the Xen heap page you use here - that would be the next question: >> Does it need to be such, rather than a domheap one? I do see ... > > I need to reference the user regs in __context_switch() before > switching to the new address space (otherwise I'd had to rework > __context_switch() which I wanted to avoid).
And a suitably mapped domain-heap page won't do? >>> + info = (struct cpu_info *)((unsigned long)ptr + PAGE_SIZE) - 1; >>> + info->flags = ON_VCPUSTACK; >>> + v->arch.pv_vcpu.stack_regs = &info->guest_cpu_user_regs; >> >> ... this pointer, but without a clear picture on intended use it's >> hard to judge. > > See patch 12. Well, that's one of the big problems with this RFC: The overview mail doesn't give a clear picture of the intended overall changes (including ones yet to be submitted), and individual patches rely on the reader to pull out information from later patches to understand what the current patch one is looking at does. >>> + /* Map TSS. */ >>> + rc = create_perdomain_mapping(d, XPTI_TSS(v), 1, NULL, &pg); >>> + if ( rc ) >>> + goto done; >>> + info = (struct cpu_info *)(XPTI_START(v) + STACK_SIZE) - 1; >> >> Iiuc this is a pointer one absolutely must not de-reference. A bit >> dangerous, I would say, the more that further up the same >> variable is being de-referenced. > > Okay, I'll add another variable for this purpose. Or at least add a comment clearly stating the restriction. Jan _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
