On 29.03.2022 16:03, Tamas K Lengyel wrote:
> Add option to the fork memop to enforce a start with an empty p2m.
> Pre-populating special pages to the fork tend to be necessary only when
> setting
> up forks to be fully functional with a toolstack or if the fork makes use of
> them in some way. For short-lived forks these pages are optional and starting
> with an empty p2m has advantages both in terms of reset performance as well as
> easier reasoning about the state of the fork after creation.
I'm afraid I don't consider this enough of an explanation: Why would these
page be optional? Where does the apriori knowledge come from that the guest
wouldn't manage to access the vCPU info pages or the APIC access one?
> --- a/xen/arch/x86/include/asm/hvm/domain.h
> +++ b/xen/arch/x86/include/asm/hvm/domain.h
> @@ -31,7 +31,9 @@
> #ifdef CONFIG_MEM_SHARING
> struct mem_sharing_domain
> {
> - bool enabled, block_interrupts;
> + bool enabled;
> + bool block_interrupts;
> + bool empty_p2m;
While the name of the field is perhaps fine as is, it would be helpful to
have a comment here clarifying that this is only about the guest's initial
and reset state; this specifically does not indicate the p2m has to remain
empty (aiui).
> @@ -1856,7 +1860,13 @@ static int fork(struct domain *cd, struct domain *d)
> if ( (rc = bring_up_vcpus(cd, d)) )
> goto done;
>
> - rc = copy_settings(cd, d);
> + if ( !(rc = copy_settings(cd, d, empty_p2m)) )
> + {
> + cd->arch.hvm.mem_sharing.block_interrupts = block_interrupts;
> +
> + if ( (cd->arch.hvm.mem_sharing.empty_p2m = empty_p2m) )
Is there a reason you don't do the assignment earlier, thus avoiding the
need to pass around the extra function argument?
> --- a/xen/include/public/memory.h
> +++ b/xen/include/public/memory.h
> @@ -543,10 +543,10 @@ struct xen_mem_sharing_op {
> } debug;
> struct mem_sharing_op_fork { /* OP_FORK */
> domid_t parent_domain; /* IN: parent's domain id */
> -/* Only makes sense for short-lived forks */
> +/* These flags only makes sense for short-lived forks */
Nit: s/makes/make/.
Jan
