From: Boris Ostrovsky <boris.ostrovsky@oracle.com>
To: "Luis R. Rodriguez" <mcgrof@suse.com>
Cc: pmonclus@plumgrid.com, GLin@suse.coma, bblanco@plumgrid.com,
x86@kernel.org, linux-kernel@vger.kernel.org,
"Luis R. Rodriguez" <mcgrof@kernel.org>,
David Vrabel <david.vrabel@citrix.com>,
hpa@zytor.com, xen-devel@lists.xenproject.org, bp@suse.de,
roger.pau@citrix.com
Subject: Re: [PATCH v2 02/11] xen/hvmlite: Bootstrap HVMlite guest
Date: Thu, 4 Feb 2016 14:54:15 -0500 [thread overview]
Message-ID: <56B3AC67.7080704__6962.94822632826$1454615745$gmane$org@oracle.com> (raw)
In-Reply-To: <20160203234026.GS20964@wotan.suse.de>
On 02/03/2016 06:40 PM, Luis R. Rodriguez wrote:
> On Wed, Feb 03, 2016 at 03:11:56PM -0500, Boris Ostrovsky wrote:
>> On 02/03/2016 01:55 PM, Luis R. Rodriguez wrote:
>>> I saw no considerations for the recommendations I had made last on your v1:
>>>
>>> https://lkml.kernel.org/r/CAB=NE6XPA0YzbnM8=rspkKai6d3GkXXO00Gr0VZUYoyzNy6thw@mail.gmail.com
>>>
>>> Of importance:
>>>
>>> 1) Using pv_info.paravirt_enabled = 1 is wrong unless you mean to say this
>>> is for legacy x86:
>>>
>>> Your patch #3 keeps on setting pv_info.paravirt_enabled = 1 and as discussed
>>> this is wrong. It will be renamed to x86_legacy_free() to align with what folks
>>> are pushing for a BIOS flag to annotate if a system requires legacy x86 stuff.
>>> This also means re-thinking all use cases and ensuring subarch is used then
>>> instead when the goal was to avoid Xen from entering that code. Today Xen does
>>> not use this but with my work it does and it helps clean and brush up a lot of
>>> these checks with future prospects to even help unify entry points.
>> As I said earlier, I am not sure I understand what subarch buys us
>> for HVMlite guests.
> I accepted subarch may not be the right thing, so proposed a hypervisor type.
I don't see much difference between having an HV-specific subarch and a
hypervisor type.
> What it buys you is a strong semantics association between code designed
> for a purpose.
>
>> As for using paravirt_enabled -- this is really only used to
>> differentiate HVM from HVMlite and I think (although I'd need to
>> check) is only needed by Xen-specific code in a couple of places.
> That sounds like a Xen specific use case as such an interface that is
> pointed out as going to renamed to reflect its actual use case should not
> be abused for that purpose.
>
>> So if/when it is removed we will switch to something else. Since your work is
>> WIP I decided to keep using it until it's clear what other options may be
>> available.
> And your work is not WIP? I'll be splitting my patches up and the rename
> will be atomic, it likely can go in first than yours, so not sure why you
> are simply brushing this off.
I didn't mean to imply anything by saying that your patches are a WIP.
It's just that I can only write and test my patches against existing
code, not the future one.
I am sorry if you felt I was trying to say something else, it certainly
was not my intent.
>
>>> 2) We should avoid more hypervisor type hacks, and just consider a new
>>> hypervisor type to close the gap:
>>>
>>> Using x86_legacy_free() and friends in a unified way for all systems means it
>>> should only be used after init_hypervisor_platform() which is called during
>>> setup_arch(). This means we have a semantic gap for checks on "are we on
>>> hypervisor type and which one?".
>> In this particular case we don't need any information about
>> hypervisor until init_hypervisor_platform().
> I pointed out in your v1 patchset how microcode loading was not blocked, you
> then asked how KVM does it, and that was explained as well, and that they
> don't enable it as well. You need a solution for this.
Not really. Xen will ignore writes to microcode-specific MSRs, just like
KVM.
This is exact same behavior we have now with regular HVM guests.
> As-is the x86 boot protocol would not allow an easy way for this, I'm
> suggesting we consider extending the boot protocol to add a hypervisor
> type and data pointer much as with subarch and subarch_data for the
Who will set hypervisor type and where? It won't be Xen as Andrew
mentioned in another email.
> particular purpose of both enabling entry into the same startup_32()
> but also a clean way for modifications of stubs both at the beginning
> and at the end of startup_32().
>
> Pseudo code:
>
> startup_32() startup_64()
> | |
> | |
> V V
> pre_hypervisor_stub_32() pre_hypervisor_stub_64()
> | |
> | |
> V V
> [existing startup_32()] [existing startup_64()]
> | |
> | |
> V V
> post_hypervisor_stub_32() post_hypervisor_stub_64()
>
> The pre_hypervisor_stub_32() would have much of the code in
> hvmlite_start_xen() but for 32-bit, pre_hypervisor_stub_64()
> would have the 64-bits.
Sure. When the protocol is agreed upon and this code is written we will
just move hvmlite_start_xen() to pre_hypervisor_stub_32().
> +int xen_hvmlite __attribute__((section(".data"))) = 0;
> +struct hvm_start_info hvmlite_start_info __attribute__((section(".data")));
> +uint hvmlite_start_info_sz = sizeof(hvmlite_start_info);
> +struct boot_params xen_hvmlite_boot_params __attribute__((section(".data")));
> +#endif
> +
>>> The section annotations seems very special use case but likely worth documenting
>>> and defining a new macro for in include/linux/compiler.h. This would make it
>>> easier to change should we want to change the section used here later and
>>> enable others to easily look for the reason for these annotations in a
>>> single place.
>> I wonder whether __initdata would be a good attribute. We only need
>> this early in the boot.
> I could not find other users of .data other than some specific driver.
> Using anything with *init* alludes you can free the data later but if we
> want to keep it I suggest a different prefix, up to you.
That's why I said that we only need this info early in the boot.
-boris
next prev parent reply other threads:[~2016-02-04 19:54 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-01 15:38 [PATCH v2 00/11] HVMlite domU support Boris Ostrovsky
2016-02-01 15:38 ` [PATCH v2 01/11] xen/hvmlite: Import hvmlite-related Xen public interfaces Boris Ostrovsky
2016-02-02 16:06 ` David Vrabel
2016-02-01 15:38 ` [PATCH v2 02/11] xen/hvmlite: Bootstrap HVMlite guest Boris Ostrovsky
2016-02-02 16:39 ` David Vrabel
[not found] ` <56B0DBD6.3060205@citrix.com>
2016-02-02 17:19 ` Boris Ostrovsky
2016-02-03 18:55 ` Luis R. Rodriguez
[not found] ` <20160203185525.GV20964@wotan.suse.de>
2016-02-03 20:11 ` Boris Ostrovsky
2016-02-03 20:52 ` Andrew Cooper
[not found] ` <56B25F0C.2050808@oracle.com>
2016-02-03 23:40 ` Luis R. Rodriguez
[not found] ` <20160203234026.GS20964@wotan.suse.de>
2016-02-04 19:54 ` Boris Ostrovsky [this message]
[not found] ` <56B3AC67.7080704@oracle.com>
2016-02-04 20:57 ` Luis R. Rodriguez
[not found] ` <20160204205721.GJ12481@wotan.suse.de>
2016-02-04 22:28 ` Boris Ostrovsky
[not found] ` <56B268A2.5000704@citrix.com>
2016-02-03 23:59 ` Luis R. Rodriguez
[not found] ` <20160203235908.GT20964@wotan.suse.de>
2016-02-04 0:08 ` Luis R. Rodriguez
2016-02-04 0:51 ` Andrew Cooper
[not found] ` <56B2A09A.70404@citrix.com>
2016-02-04 23:10 ` Luis R. Rodriguez
[not found] ` <20160204231031.GM12481@wotan.suse.de>
2016-04-05 22:02 ` Luis R. Rodriguez
2016-04-24 20:23 ` Borislav Petkov
[not found] ` <20160424202314.GA3973@pd.tnic>
2016-04-25 13:21 ` Boris Ostrovsky
[not found] ` <571E19D7.1080301@oracle.com>
2016-04-25 13:47 ` Borislav Petkov
[not found] ` <20160425134749.GB28454@pd.tnic>
2016-04-25 13:54 ` Boris Ostrovsky
[not found] ` <571E219D.2090308@oracle.com>
2016-04-25 14:11 ` Borislav Petkov
[not found] ` <20160425141145.GE28454@pd.tnic>
2016-04-25 14:42 ` Boris Ostrovsky
[not found] ` <571E2CC7.7080907@oracle.com>
2016-04-25 15:22 ` Borislav Petkov
[not found] ` <20160425152209.GH28454@pd.tnic>
2016-04-25 15:48 ` Boris Ostrovsky
2016-04-26 10:53 ` Borislav Petkov
2016-04-25 17:24 ` David Vrabel
2016-02-01 15:38 ` [PATCH v2 03/11] xen/hvmlite: Initialize HVMlite kernel Boris Ostrovsky
2016-02-17 20:08 ` Luis R. Rodriguez
2016-02-01 15:38 ` [PATCH v2 04/11] xen/hvmlite: Allow HVMlite guests delay initializing grant table Boris Ostrovsky
2016-02-01 15:38 ` [PATCH v2 05/11] xen/hvmlite: HVMlite guests always have PV devices Boris Ostrovsky
2016-02-01 15:38 ` [PATCH v2 06/11] xen/hvmlite: Prepare cpu_initialize_context() routine for HVMlite SMP Boris Ostrovsky
2016-02-02 16:16 ` David Vrabel
2016-02-01 15:38 ` [PATCH v2 07/11] xen/hvmlite: Initialize context for secondary VCPUs Boris Ostrovsky
2016-02-02 16:21 ` David Vrabel
[not found] ` <56B0D786.7000002@citrix.com>
2016-02-02 16:58 ` Boris Ostrovsky
[not found] ` <56B0E046.6050900@oracle.com>
2016-02-04 10:07 ` David Vrabel
2016-02-04 12:58 ` Doug Goldstein
[not found] ` <56B34B01.50000@cardoe.com>
2016-02-04 19:08 ` Boris Ostrovsky
2016-02-01 15:38 ` [PATCH v2 08/11] xen/hvmlite: Extend APIC operations for HVMlite guests Boris Ostrovsky
2016-02-04 10:04 ` David Vrabel
[not found] ` <56B32220.4040505@citrix.com>
2016-02-04 12:14 ` Roger Pau Monné
[not found] ` <56B340B4.3050406@citrix.com>
2016-02-04 14:01 ` Boris Ostrovsky
2016-02-01 15:38 ` [PATCH v2 09/11] xen/hvmlite: Use x86's default timer init " Boris Ostrovsky
2016-02-02 16:27 ` David Vrabel
[not found] ` <56B0D8DD.5010206@citrix.com>
2016-02-02 17:01 ` Boris Ostrovsky
2016-02-01 15:38 ` [PATCH v2 10/11] xen/hvmlite: Boot secondary CPUs Boris Ostrovsky
2016-02-04 10:38 ` David Vrabel
[not found] ` <56B32A29.6050406@citrix.com>
2016-02-04 13:51 ` Boris Ostrovsky
2016-02-01 15:38 ` [PATCH v2 11/11] xen/hvmlite: Enable CPU on-/offlining Boris Ostrovsky
[not found] ` <1454341137-14110-5-git-send-email-boris.ostrovsky@oracle.com>
2016-02-02 16:13 ` [PATCH v2 04/11] xen/hvmlite: Allow HVMlite guests delay initializing grant table David Vrabel
[not found] ` <56B0D596.4050301@citrix.com>
2016-02-02 16:49 ` Boris Ostrovsky
2016-02-03 18:59 ` Luis R. Rodriguez
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='56B3AC67.7080704__6962.94822632826$1454615745$gmane$org@oracle.com' \
--to=boris.ostrovsky@oracle.com \
--cc=GLin@suse.coma \
--cc=bblanco@plumgrid.com \
--cc=bp@suse.de \
--cc=david.vrabel@citrix.com \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mcgrof@kernel.org \
--cc=mcgrof@suse.com \
--cc=pmonclus@plumgrid.com \
--cc=roger.pau@citrix.com \
--cc=x86@kernel.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).