Re: [edk2-devel] [RFC PATCH 01/28] OvmfPkg/Sec: Enable cache early to speed up booting
On 08/22/19 22:44, Jordan Justen wrote:
> On 2019-08-22 06:46:07, Laszlo Ersek wrote:
>> On 08/21/19 23:51, Jordan Justen wrote:
>>> On 2019-08-21 07:21:25, Laszlo Ersek wrote:
On 08/19/19 23:35, Lendacky, Thomas wrote:
> From: Tom Lendacky
>
> + //
> + // Enable caching
> + //
> + AsmEnableCache ();
> +
>DEBUG ((EFI_D_INFO,
> "SecCoreStartupWithStack(0x%x, 0x%x)\n",
> (UINT32)(UINTN)BootFv,
>
This makes me uncomfortable. There used to be problems related to
caching when VFIO device assignment were used. My concern is admittedly
vague, but this is a very brittle area of OVMF-on-KVM. If you asked me
"well what could break here", I'd answer "you never know, and the burden
of proof is not on me". :) Can we make this change conditional on SEV-ES?
>>>
>>> This was also raised as an issue by Peter for the ACRN hypervisor and
>>> Scott for the bhyve hypervisor.
>>>
>>> I think it is rare for a platform to enable cache at this early of a
>>> stage, but it is also rare to decompress a firmware volume at this
>>> point.
>>>
>>> It appears that it could be helpful to figure out how to safely enable
>>> cache by default here, since it does seem to be impacting several
>>> hypervisors.
>>
>> I can't think of anything better than "trial and error".
>
> Maybe we could try to detect kvm, and enable caching if !kvm.
>
> Maybe we could enable it during the decompress of the PEI FV and
> disable it afterward?
>
>> The issues that
>> used to pop up in the past, due to host kernel (KVM) changes,
>> particularly in connection with VFIO device assignment, have been
>> completely obscure and unpenetrable to me.
>
> Don't we eventually enable caching during the boot, so how is VFIO not
> affected by that?
>
>> Even though I've contributed
>> at least one KVM patch to mitigate those problems, they remain a mistery
>> to me, and I remain unable to *reason* about the problems or the fixes.
>
> If VFIO requires uncached access, then what mechanisms does kvm
> support for accessing memory ranges uncached? I thought kvm simply
> ignored the cache setting and always enabled caching, because this
> section of boot is not particularly slow with kvm. But, if enabling
> caching causes issues, then I guess it does something.
>
> Does kvm support mtrr to uncache i/o ranges? I didn't think kvm
> supported mtrrs in the past.
>
> I hope we wouldn't have to use paging to disable caching for the
> affected regions.
All good questions, and I'm not equipped to answer them, unfortunately.
I'm happy to review and regression-test -- including GPU assignment, on
my workstation that's dedicated to that use case -- OvmfPkg patches, if
someone posts them.
This stuff is in constant flux in KVM. Recent example:
https://www.redhat.com/archives/vfio-users/2019-August/msg00016.html
Thanks
Laszlo
>> So I think we could only flip the behavior (enable cache by default) and
>> collect bug reports. But that's extremely annoying for end-users, and I
>> see "no regressions" as one of my top responsibilities.
>>
>> Even if we provided an fw_cfg knob to disable the change, using
>> QemuFwCfgSecLib, similarly to commit ab081a50e565 ("OvmfPkg:
>> PlatformPei: take no-exec DXE settings from the QEMU command line",
>> 2015-09-15), such fw_cfg knobs are difficult to use through layered
>> products, such as libvirt, proxmox, etc. And of course fw_cfg is only
>> available on QEMU.
>>
>> Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#46303): https://edk2.groups.io/g/devel/message/46303
Mute This Topic: https://groups.io/mt/32966275/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [RFC PATCH 01/28] OvmfPkg/Sec: Enable cache early to speed up booting
On 2019-08-22 06:46:07, Laszlo Ersek wrote:
> On 08/21/19 23:51, Jordan Justen wrote:
> > On 2019-08-21 07:21:25, Laszlo Ersek wrote:
> >> On 08/19/19 23:35, Lendacky, Thomas wrote:
> >>> From: Tom Lendacky
> >>>
> >>> + //
> >>> + // Enable caching
> >>> + //
> >>> + AsmEnableCache ();
> >>> +
> >>>DEBUG ((EFI_D_INFO,
> >>> "SecCoreStartupWithStack(0x%x, 0x%x)\n",
> >>> (UINT32)(UINTN)BootFv,
> >>>
> >>
> >> This makes me uncomfortable. There used to be problems related to
> >> caching when VFIO device assignment were used. My concern is admittedly
> >> vague, but this is a very brittle area of OVMF-on-KVM. If you asked me
> >> "well what could break here", I'd answer "you never know, and the burden
> >> of proof is not on me". :) Can we make this change conditional on SEV-ES?
> >
> > This was also raised as an issue by Peter for the ACRN hypervisor and
> > Scott for the bhyve hypervisor.
> >
> > I think it is rare for a platform to enable cache at this early of a
> > stage, but it is also rare to decompress a firmware volume at this
> > point.
> >
> > It appears that it could be helpful to figure out how to safely enable
> > cache by default here, since it does seem to be impacting several
> > hypervisors.
>
> I can't think of anything better than "trial and error".
Maybe we could try to detect kvm, and enable caching if !kvm.
Maybe we could enable it during the decompress of the PEI FV and
disable it afterward?
> The issues that
> used to pop up in the past, due to host kernel (KVM) changes,
> particularly in connection with VFIO device assignment, have been
> completely obscure and unpenetrable to me.
Don't we eventually enable caching during the boot, so how is VFIO not
affected by that?
> Even though I've contributed
> at least one KVM patch to mitigate those problems, they remain a mistery
> to me, and I remain unable to *reason* about the problems or the fixes.
If VFIO requires uncached access, then what mechanisms does kvm
support for accessing memory ranges uncached? I thought kvm simply
ignored the cache setting and always enabled caching, because this
section of boot is not particularly slow with kvm. But, if enabling
caching causes issues, then I guess it does something.
Does kvm support mtrr to uncache i/o ranges? I didn't think kvm
supported mtrrs in the past.
I hope we wouldn't have to use paging to disable caching for the
affected regions.
-Jordan
> So I think we could only flip the behavior (enable cache by default) and
> collect bug reports. But that's extremely annoying for end-users, and I
> see "no regressions" as one of my top responsibilities.
>
> Even if we provided an fw_cfg knob to disable the change, using
> QemuFwCfgSecLib, similarly to commit ab081a50e565 ("OvmfPkg:
> PlatformPei: take no-exec DXE settings from the QEMU command line",
> 2015-09-15), such fw_cfg knobs are difficult to use through layered
> products, such as libvirt, proxmox, etc. And of course fw_cfg is only
> available on QEMU.
>
> Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#46236): https://edk2.groups.io/g/devel/message/46236
Mute This Topic: https://groups.io/mt/32966275/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [RFC PATCH 01/28] OvmfPkg/Sec: Enable cache early to speed up booting
On 08/21/19 23:51, Jordan Justen wrote:
> On 2019-08-21 07:21:25, Laszlo Ersek wrote:
>> On 08/19/19 23:35, Lendacky, Thomas wrote:
>>> From: Tom Lendacky
>>>
>>> Currently, the OVMF code relies on the hypervisor to enable the cache
>>> support on the processor in order to improve the boot speed. However,
>>> with SEV-ES, the hypervisor is not allowed to change the CR0 register
>>> to enable caching.
>>>
>>> Update the OVMF Sec support to enable caching in order to improve the
>>> boot speed.
>>>
>>> Signed-off-by: Tom Lendacky
>>> ---
>>> OvmfPkg/Sec/SecMain.c | 5 +
>>> 1 file changed, 5 insertions(+)
>>>
>>> diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c
>>> index 3914355cd17b..2448be0cd408 100644
>>> --- a/OvmfPkg/Sec/SecMain.c
>>> +++ b/OvmfPkg/Sec/SecMain.c
>>> @@ -739,6 +739,11 @@ SecCoreStartupWithStack (
>>>
>>>ProcessLibraryConstructorList (NULL, NULL);
>>>
>>> + //
>>> + // Enable caching
>>> + //
>>> + AsmEnableCache ();
>>> +
>>>DEBUG ((EFI_D_INFO,
>>> "SecCoreStartupWithStack(0x%x, 0x%x)\n",
>>> (UINT32)(UINTN)BootFv,
>>>
>>
>> This makes me uncomfortable. There used to be problems related to
>> caching when VFIO device assignment were used. My concern is admittedly
>> vague, but this is a very brittle area of OVMF-on-KVM. If you asked me
>> "well what could break here", I'd answer "you never know, and the burden
>> of proof is not on me". :) Can we make this change conditional on SEV-ES?
>
> This was also raised as an issue by Peter for the ACRN hypervisor and
> Scott for the bhyve hypervisor.
>
> I think it is rare for a platform to enable cache at this early of a
> stage, but it is also rare to decompress a firmware volume at this
> point.
>
> It appears that it could be helpful to figure out how to safely enable
> cache by default here, since it does seem to be impacting several
> hypervisors.
I can't think of anything better than "trial and error". The issues that
used to pop up in the past, due to host kernel (KVM) changes,
particularly in connection with VFIO device assignment, have been
completely obscure and unpenetrable to me. Even though I've contributed
at least one KVM patch to mitigate those problems, they remain a mistery
to me, and I remain unable to *reason* about the problems or the fixes.
So I think we could only flip the behavior (enable cache by default) and
collect bug reports. But that's extremely annoying for end-users, and I
see "no regressions" as one of my top responsibilities.
Even if we provided an fw_cfg knob to disable the change, using
QemuFwCfgSecLib, similarly to commit ab081a50e565 ("OvmfPkg:
PlatformPei: take no-exec DXE settings from the QEMU command line",
2015-09-15), such fw_cfg knobs are difficult to use through layered
products, such as libvirt, proxmox, etc. And of course fw_cfg is only
available on QEMU.
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#46211): https://edk2.groups.io/g/devel/message/46211
Mute This Topic: https://groups.io/mt/32966275/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [RFC PATCH 01/28] OvmfPkg/Sec: Enable cache early to speed up booting
On 2019-08-21 07:21:25, Laszlo Ersek wrote: > On 08/19/19 23:35, Lendacky, Thomas wrote: > > From: Tom Lendacky > > > > Currently, the OVMF code relies on the hypervisor to enable the cache > > support on the processor in order to improve the boot speed. However, > > with SEV-ES, the hypervisor is not allowed to change the CR0 register > > to enable caching. > > > > Update the OVMF Sec support to enable caching in order to improve the > > boot speed. > > > > Signed-off-by: Tom Lendacky > > --- > > OvmfPkg/Sec/SecMain.c | 5 + > > 1 file changed, 5 insertions(+) > > > > diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c > > index 3914355cd17b..2448be0cd408 100644 > > --- a/OvmfPkg/Sec/SecMain.c > > +++ b/OvmfPkg/Sec/SecMain.c > > @@ -739,6 +739,11 @@ SecCoreStartupWithStack ( > > > >ProcessLibraryConstructorList (NULL, NULL); > > > > + // > > + // Enable caching > > + // > > + AsmEnableCache (); > > + > >DEBUG ((EFI_D_INFO, > > "SecCoreStartupWithStack(0x%x, 0x%x)\n", > > (UINT32)(UINTN)BootFv, > > > > This makes me uncomfortable. There used to be problems related to > caching when VFIO device assignment were used. My concern is admittedly > vague, but this is a very brittle area of OVMF-on-KVM. If you asked me > "well what could break here", I'd answer "you never know, and the burden > of proof is not on me". :) Can we make this change conditional on SEV-ES? This was also raised as an issue by Peter for the ACRN hypervisor and Scott for the bhyve hypervisor. I think it is rare for a platform to enable cache at this early of a stage, but it is also rare to decompress a firmware volume at this point. It appears that it could be helpful to figure out how to safely enable cache by default here, since it does seem to be impacting several hypervisors. -Jordan -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#46183): https://edk2.groups.io/g/devel/message/46183 Mute This Topic: https://groups.io/mt/32966275/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [RFC PATCH 01/28] OvmfPkg/Sec: Enable cache early to speed up booting
On 8/21/19 9:21 AM, Laszlo Ersek wrote: > On 08/19/19 23:35, Lendacky, Thomas wrote: >> From: Tom Lendacky >> >> Currently, the OVMF code relies on the hypervisor to enable the cache >> support on the processor in order to improve the boot speed. However, >> with SEV-ES, the hypervisor is not allowed to change the CR0 register >> to enable caching. >> >> Update the OVMF Sec support to enable caching in order to improve the >> boot speed. >> >> Signed-off-by: Tom Lendacky >> --- >> OvmfPkg/Sec/SecMain.c | 5 + >> 1 file changed, 5 insertions(+) >> >> diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c >> index 3914355cd17b..2448be0cd408 100644 >> --- a/OvmfPkg/Sec/SecMain.c >> +++ b/OvmfPkg/Sec/SecMain.c >> @@ -739,6 +739,11 @@ SecCoreStartupWithStack ( >> >>ProcessLibraryConstructorList (NULL, NULL); >> >> + // >> + // Enable caching >> + // >> + AsmEnableCache (); >> + >>DEBUG ((EFI_D_INFO, >> "SecCoreStartupWithStack(0x%x, 0x%x)\n", >> (UINT32)(UINTN)BootFv, >> > > This makes me uncomfortable. There used to be problems related to > caching when VFIO device assignment were used. My concern is admittedly > vague, but this is a very brittle area of OVMF-on-KVM. If you asked me > "well what could break here", I'd answer "you never know, and the burden > of proof is not on me". :) Can we make this change conditional on SEV-ES? > I'll look into that. Anything is possible, just might have to read an MSR at this stage. Thanks, Tom > Thanks > Laszlo > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#46178): https://edk2.groups.io/g/devel/message/46178 Mute This Topic: https://groups.io/mt/32966275/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [RFC PATCH 01/28] OvmfPkg/Sec: Enable cache early to speed up booting
On 08/19/19 23:35, Lendacky, Thomas wrote: > From: Tom Lendacky > > Currently, the OVMF code relies on the hypervisor to enable the cache > support on the processor in order to improve the boot speed. However, > with SEV-ES, the hypervisor is not allowed to change the CR0 register > to enable caching. > > Update the OVMF Sec support to enable caching in order to improve the > boot speed. > > Signed-off-by: Tom Lendacky > --- > OvmfPkg/Sec/SecMain.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c > index 3914355cd17b..2448be0cd408 100644 > --- a/OvmfPkg/Sec/SecMain.c > +++ b/OvmfPkg/Sec/SecMain.c > @@ -739,6 +739,11 @@ SecCoreStartupWithStack ( > >ProcessLibraryConstructorList (NULL, NULL); > > + // > + // Enable caching > + // > + AsmEnableCache (); > + >DEBUG ((EFI_D_INFO, > "SecCoreStartupWithStack(0x%x, 0x%x)\n", > (UINT32)(UINTN)BootFv, > This makes me uncomfortable. There used to be problems related to caching when VFIO device assignment were used. My concern is admittedly vague, but this is a very brittle area of OVMF-on-KVM. If you asked me "well what could break here", I'd answer "you never know, and the burden of proof is not on me". :) Can we make this change conditional on SEV-ES? Thanks Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#46156): https://edk2.groups.io/g/devel/message/46156 Mute This Topic: https://groups.io/mt/32966275/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [RFC PATCH 01/28] OvmfPkg/Sec: Enable cache early to speed up booting
From: Tom Lendacky Currently, the OVMF code relies on the hypervisor to enable the cache support on the processor in order to improve the boot speed. However, with SEV-ES, the hypervisor is not allowed to change the CR0 register to enable caching. Update the OVMF Sec support to enable caching in order to improve the boot speed. Signed-off-by: Tom Lendacky --- OvmfPkg/Sec/SecMain.c | 5 + 1 file changed, 5 insertions(+) diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c index 3914355cd17b..2448be0cd408 100644 --- a/OvmfPkg/Sec/SecMain.c +++ b/OvmfPkg/Sec/SecMain.c @@ -739,6 +739,11 @@ SecCoreStartupWithStack ( ProcessLibraryConstructorList (NULL, NULL); + // + // Enable caching + // + AsmEnableCache (); + DEBUG ((EFI_D_INFO, "SecCoreStartupWithStack(0x%x, 0x%x)\n", (UINT32)(UINTN)BootFv, -- 2.17.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#46103): https://edk2.groups.io/g/devel/message/46103 Mute This Topic: https://groups.io/mt/32966275/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
