[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 2/9] x86/PV: properly set shadow allocation for Dom0

  • To: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Wed, 22 Sep 2021 15:50:25 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=VTFda4+SGUG0aGYL1bV9SUmIq1XXpALnAtoQXqLCOqE=; b=BweWMUnheQ2VbrrqSClhnE85rKNnDpI+tQggz1P5eteqv6R0GKYYt+Q/0nUxcJODGE9aW+m80Tg3kVFW1AO4Vl4o1u/ba2uJk28GriBfcrP84DskCZGewSRiBArCN8ZVEPxKsv9qdeLRo45FXSrtXW5pAAw06FK4ilzdXKvcCo6f3AEWJ3Wwy7C88VcVpBMvgkiaH3O8B9KfpvHX6dlScH0gQiOjmaNOSvxZex+UZjDqHc9fXTiLQ9cRhMKJmuqRF3VxQG2nkVGW68wY5eSyyzf+XsGaA5N0jTtJQgYjODjqP6N3qQRW+hecbjLm1arQRTLaa0Eryg2sVZRYGUftUg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=YFPeItqSdl0dF9QsH3ibQBSM7aSbbLMB+WvvT33ExnKbvtgDXz0FvKIhv1ENTtK1JEKZS/8MF2lNzViV+CRlbGh3WibonW0pt7l9f+sCHBPSow0L5q5xPJ5JDDHcE0/uZ5qHa+fe41u+Ayr1fw1v0pBQWbdwLOQ4PxvLahCBZ78uTtngNujWUMOKIt4OF7yZr3spYP3jNYT6xViH3s2Ih2nkkasbZLYg8YvY+Nk+aTXhRI7bXCWLEwvHHjldeMtDC8oAPYJKDYHg/eiieT5D21ykKvVtGzcpyt068dym0Z/4s9H59eLgppZoN1KE8zGOEmu9b+RB8SIyKT86hfuyrw==
  • Authentication-results: lists.xenproject.org; dkim=none (message not signed) header.d=none;lists.xenproject.org; dmarc=none action=none header.from=suse.com;
  • Cc: Wei Liu <wl@xxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Tim Deegan <tim@xxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Wed, 22 Sep 2021 13:50:37 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 22.09.2021 15:31, Andrew Cooper wrote:
> On 21/09/2021 08:17, Jan Beulich wrote:
>> @@ -933,7 +934,18 @@ int __init dom0_construct_pv(struct doma
>>      if ( opt_dom0_shadow )
>>      {
>> +        bool preempted;
>> +
>>          printk("Switching dom0 to using shadow paging\n");
>> +
>> +        nr_pt_pages = dom0_paging_pages(d, nr_pages);
>> +
>> +        do {
>> +            preempted = false;
>> +            shadow_set_allocation(d, nr_pt_pages, &preempted);
>> +            process_pending_softirqs();
>> +        } while ( preempted );
> This is still broken.
> The loop setting the shadow allocation needs to be outside of this
> conditional, because it is not related to early activation of the l1tf
> tasklet.

Well, I'm not sure what to say. On v1 you already said so. But then you
didn't care to reply to me responding:

"Are you suggesting to set up a (perhaps large) shadow pool just in
 case we need to enable shadow mode on Dom0? And all of this memory
 to then remain unused in the majority of cases?

 Plus even if so, I'd view this as a 2nd, independent step, largely
 orthogonal to the handling of "dom0=shadow". If somebody really
 wanted that, I think this should be driven by an explicit setting
 of the shadow pool size, indicating the admin is willing to waste
 the memory.

 I'm further puzzled by "not to retain upstream's security
 vulnerability" - are you saying upstream is vulnerable in some way,
 while perhaps you (XenServer) are not? In general I don't think I
 view downstream decisions as a driving factor for what upstream
 does, when the result is deliberately different behavior from

Which has left me with no justification to make the change you're
requesting. I've now got an ack by Tim and an R-b by Roger. I also
view the change as is being an improvement on its own (i.e. I
question you saying "This is still broken."), even if (later) we
were to follow what you request. For this reason I'll give it a day
or two for you to reply, but otherwise I'll commit the patch as is,
leaving further adjustments for a future change (by you, me, or
anyone else).




Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.