From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754741AbbINLQO (ORCPT ); Mon, 14 Sep 2015 07:16:14 -0400 Received: from mail-io0-f172.google.com ([209.85.223.172]:36296 "EHLO mail-io0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753234AbbINLQM (ORCPT ); Mon, 14 Sep 2015 07:16:12 -0400 MIME-Version: 1.0 In-Reply-To: <55F6C01502000078000A2823@prv-mh.provo.novell.com> References: <20150910112418.GC29293@leverpostej> <20150910121514.GE29293@leverpostej> <20150910144938.GI29293@leverpostej> <20150910162302.GN29293@leverpostej> <20150911124643.GB4530@olila.local.net-space.pl> <20150911154534.GD4530@olila.local.net-space.pl> <55F6886D.1090900@huawei.com> <55F693F5.8030203@huawei.com> <55F6C01502000078000A2823@prv-mh.provo.novell.com> Date: Mon, 14 Sep 2015 13:16:11 +0200 Message-ID: Subject: Re: [Xen-devel] [PATCH] efi/libstub/fdt: Standardize the names of EFI stub parameters From: Ard Biesheuvel To: Jan Beulich Cc: Mark Rutland , "Ian.Campbell@citrix.com" , "julien.grall@citrix.com" , Stefano Stabellini , Shannon Zhao , "leif.lindholm@linaro.org" , "shannon.zhao@linaro.org" , "linux-arm-kernel@lists.infradead.org" , "xen-devel@lists.xen.org" , Daniel Kiper , "linux-efi@vger.kernel.org" , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 14 September 2015 at 12:39, Jan Beulich wrote: >>>> On 14.09.15 at 11:36, wrote: >> On 14 September 2015 at 11:31, Shannon Zhao wrote: >>> My understanding is that if there are no EFI_MEMORY_RUNTIME regions, it >>> means we can't use runtime services and should not set the bit >>> EFI_RUNTIME_SERVICES of efi.flags. But if efi_virtmap_init() return >>> true, the bit will be set. >>> >> >> As I said, if you don't want the EFI_RUNTIME_SERVICES bit to be set >> for other reasons, don't rig efi_virtmap_init() to return false when >> it shouldn't. >> >>>> The absence of such regions is allowed by the spec, so >>>> efi_virtmap_init() is correct imo to return success. >>>> >>> Sorry, not well know about the spec. Could you point out where the spec >>> says this? >>> >> >> Well, I think it doesn't work that way. You are claiming that a memory >> map without at least one EFI_MEMORY_RUNTIME constitutes an error >> condition, so the burden is on you to provide a reference to the spec >> that says you must have at least one such region. > > Sure, from a spec pov you're right. But where would runtime > services code/data live when there's not a single region marked > as needing a runtime mapping. IOW while the spec doesn't say > so, assuming no runtime services when there's not at least one > executable region with the runtime flag set could serve as a stop > gap measure against flawed firmware. > That in itself is a fair point. But the argument being made was that it is in itself a bug for no EFI_MEMORY_RUNTIME regions to exist, while the actual purpose of the patch is to prevent the RuntimeServices pointer from being dereferenced on platforms like Xen that may not be able to implement them. But actually, even in case of Xen, you will need some EFI_MEMORY_RUNTIME regions anyway, since the f/w vendor field and the config and runtime services table pointers are translated to virtual addresses by the firmware, which means the OS needs to translate them back to physical addresses in order to dereference them before the VA mapping is up. (I still think not using SetVirtualAddressMap() at all would be the best approach for arm64, but unfortunately, most of my colleagues disagree with me) -- Ard. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ard Biesheuvel Subject: Re: [Xen-devel] [PATCH] efi/libstub/fdt: Standardize the names of EFI stub parameters Date: Mon, 14 Sep 2015 13:16:11 +0200 Message-ID: References: <20150910112418.GC29293@leverpostej> <20150910121514.GE29293@leverpostej> <20150910144938.GI29293@leverpostej> <20150910162302.GN29293@leverpostej> <20150911124643.GB4530@olila.local.net-space.pl> <20150911154534.GD4530@olila.local.net-space.pl> <55F6886D.1090900@huawei.com> <55F693F5.8030203@huawei.com> <55F6C01502000078000A2823@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Return-path: In-Reply-To: <55F6C01502000078000A2823-rw/UEucdPrvD8XXLLHKrIiOjQekVJEpY@public.gmane.org> Sender: linux-efi-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Jan Beulich Cc: Mark Rutland , "Ian.Campbell-Sxgqhf6Nn4DQT0dZR+AlfA@public.gmane.org" , "julien.grall-Sxgqhf6Nn4DQT0dZR+AlfA@public.gmane.org" , Stefano Stabellini , Shannon Zhao , "leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org" , "shannon.zhao-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org" , "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" , "xen-devel-GuqFBffKawuEi8DpZVb4nw@public.gmane.org" , Daniel Kiper , "linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" List-Id: linux-efi@vger.kernel.org On 14 September 2015 at 12:39, Jan Beulich wrote: >>>> On 14.09.15 at 11:36, wrote: >> On 14 September 2015 at 11:31, Shannon Zhao wrote: >>> My understanding is that if there are no EFI_MEMORY_RUNTIME regions, it >>> means we can't use runtime services and should not set the bit >>> EFI_RUNTIME_SERVICES of efi.flags. But if efi_virtmap_init() return >>> true, the bit will be set. >>> >> >> As I said, if you don't want the EFI_RUNTIME_SERVICES bit to be set >> for other reasons, don't rig efi_virtmap_init() to return false when >> it shouldn't. >> >>>> The absence of such regions is allowed by the spec, so >>>> efi_virtmap_init() is correct imo to return success. >>>> >>> Sorry, not well know about the spec. Could you point out where the spec >>> says this? >>> >> >> Well, I think it doesn't work that way. You are claiming that a memory >> map without at least one EFI_MEMORY_RUNTIME constitutes an error >> condition, so the burden is on you to provide a reference to the spec >> that says you must have at least one such region. > > Sure, from a spec pov you're right. But where would runtime > services code/data live when there's not a single region marked > as needing a runtime mapping. IOW while the spec doesn't say > so, assuming no runtime services when there's not at least one > executable region with the runtime flag set could serve as a stop > gap measure against flawed firmware. > That in itself is a fair point. But the argument being made was that it is in itself a bug for no EFI_MEMORY_RUNTIME regions to exist, while the actual purpose of the patch is to prevent the RuntimeServices pointer from being dereferenced on platforms like Xen that may not be able to implement them. But actually, even in case of Xen, you will need some EFI_MEMORY_RUNTIME regions anyway, since the f/w vendor field and the config and runtime services table pointers are translated to virtual addresses by the firmware, which means the OS needs to translate them back to physical addresses in order to dereference them before the VA mapping is up. (I still think not using SetVirtualAddressMap() at all would be the best approach for arm64, but unfortunately, most of my colleagues disagree with me) -- Ard. From mboxrd@z Thu Jan 1 00:00:00 1970 From: ard.biesheuvel@linaro.org (Ard Biesheuvel) Date: Mon, 14 Sep 2015 13:16:11 +0200 Subject: [Xen-devel] [PATCH] efi/libstub/fdt: Standardize the names of EFI stub parameters In-Reply-To: <55F6C01502000078000A2823@prv-mh.provo.novell.com> References: <20150910112418.GC29293@leverpostej> <20150910121514.GE29293@leverpostej> <20150910144938.GI29293@leverpostej> <20150910162302.GN29293@leverpostej> <20150911124643.GB4530@olila.local.net-space.pl> <20150911154534.GD4530@olila.local.net-space.pl> <55F6886D.1090900@huawei.com> <55F693F5.8030203@huawei.com> <55F6C01502000078000A2823@prv-mh.provo.novell.com> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 14 September 2015 at 12:39, Jan Beulich wrote: >>>> On 14.09.15 at 11:36, wrote: >> On 14 September 2015 at 11:31, Shannon Zhao wrote: >>> My understanding is that if there are no EFI_MEMORY_RUNTIME regions, it >>> means we can't use runtime services and should not set the bit >>> EFI_RUNTIME_SERVICES of efi.flags. But if efi_virtmap_init() return >>> true, the bit will be set. >>> >> >> As I said, if you don't want the EFI_RUNTIME_SERVICES bit to be set >> for other reasons, don't rig efi_virtmap_init() to return false when >> it shouldn't. >> >>>> The absence of such regions is allowed by the spec, so >>>> efi_virtmap_init() is correct imo to return success. >>>> >>> Sorry, not well know about the spec. Could you point out where the spec >>> says this? >>> >> >> Well, I think it doesn't work that way. You are claiming that a memory >> map without at least one EFI_MEMORY_RUNTIME constitutes an error >> condition, so the burden is on you to provide a reference to the spec >> that says you must have at least one such region. > > Sure, from a spec pov you're right. But where would runtime > services code/data live when there's not a single region marked > as needing a runtime mapping. IOW while the spec doesn't say > so, assuming no runtime services when there's not at least one > executable region with the runtime flag set could serve as a stop > gap measure against flawed firmware. > That in itself is a fair point. But the argument being made was that it is in itself a bug for no EFI_MEMORY_RUNTIME regions to exist, while the actual purpose of the patch is to prevent the RuntimeServices pointer from being dereferenced on platforms like Xen that may not be able to implement them. But actually, even in case of Xen, you will need some EFI_MEMORY_RUNTIME regions anyway, since the f/w vendor field and the config and runtime services table pointers are translated to virtual addresses by the firmware, which means the OS needs to translate them back to physical addresses in order to dereference them before the VA mapping is up. (I still think not using SetVirtualAddressMap() at all would be the best approach for arm64, but unfortunately, most of my colleagues disagree with me) -- Ard.