Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <2a1a9e8c-0635-e207-e858-0e0bd1df0f11@suse.com>
Date:   Tue, 6 Sep 2022 08:49:54 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.2.1
Subject: Re: [PATCH] Add support for ESRT loading under Xen
Content-Language: en-US
References: <20220825215218.1606-1-demi@invisiblethingslab.com>
 <c2a22672-b9dd-7aa4-b61e-ccb0faaa3b01@suse.com> <YwkKiFIKHG4IcCmH@itl-email>
Cc:     linux-efi@vger.kernel.org, linux-kernel@vger.kernel.org,
        xen-devel@lists.xenproject.org, Ard Biesheuvel <ardb@kernel.org>,
        Juergen Gross <jgross@suse.com>,
        Stefano Stabellini <sstabellini@kernel.org>,
        Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
From:   Jan Beulich <jbeulich@suse.com>
To:     Demi Marie Obenour <demi@invisiblethingslab.com>
In-Reply-To: <YwkKiFIKHG4IcCmH@itl-email>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?Yzhpc2d2K04zMDYyZDUrZ0JZTXFFWFlzL1c4Um5sNHpvQjdvSmhHVk5KRmEz?=
 =?utf-8?B?cHN3MmxSQTVSbnI0elp4R0U4cEhBT2NnanA5eEpQSzhSOFd5OGNoRFVEUGZP?=
 =?utf-8?B?eXlpMFV4WlBXcnhnU2ZCRndxMkl5RWtoOGR5YUVuUjZpYzlnUkhTL1VhZVhU?=
 =?utf-8?B?dTJFTmhabnJNL1lxNFF4eUFqVURNSzFkY3lia1hnQ0JWdUFEbXJFTnRuQi8r?=
 =?utf-8?B?M082bUNxWmtwdUdaOExSR1loZ29rVWlmL2tmTVY2ck9NTXJpK3FmR3k1UmNI?=
 =?utf-8?B?VW13Wm5rY3FkNjM0T3VudFhvUkJQcDNUcHpnbDFsTXQwM2I3dk1hcGRYT2Vj?=
 =?utf-8?B?dFVmdVZwZWFVSHdKbzJ3ajg3UWtUVHF6Z0x6K3BSdVF1Y040bXE1LzVnQ0dV?=
 =?utf-8?B?QjNVSG1HRmhBMk8vQm1vNTI5bXpnTGVJMTl4eWdKenppTGV5N1h2cjZkUFFO?=
 =?utf-8?B?R3BITzNlemdOcXp6OEQ3ZzYyQy9wQjdFTHFmdGx4MVM0WFhoNUhydGtPcVRV?=
 =?utf-8?B?RnhQYUJnTnA3QVNZMi9HbjdhNTJWNktYOGJURzFlRE1YTVlpRnVKc05ISGJB?=
 =?utf-8?B?VDl1MCt5c09qYlpuREZyZXVieVNrNlVjOGpzS1hTU1g4eE11S3Jmb3ZMMGRN?=
 =?utf-8?B?ZG5peitESEdzdU5rY084Z1NzOVR4d2JBQVRzbzJqdlRPSDJYYWJvTDdXRnJ1?=
 =?utf-8?B?cWZCa0ZPeVFINXk3S0srK3BnUVRVS2Q4YldEODJpMGlPa3VpS0Q4VTNPcjVL?=
 =?utf-8?B?VG9NRko5RDRBTVJCaHVESWFRczlzUGhua0wzYm9yazdsMXlYd1ZRS2Z0Uy9s?=
 =?utf-8?B?blJsOWVEd3hNaEp4L1pNOVMyaWYwcExRbE45ZHhlczB0L2VPclV3NElQVjJL?=
 =?utf-8?B?NjQ2SncrYUtOL3BTejVudndETytyS09oc090Rkk3SHkxd1c5QWptZFJLK3JI?=
 =?utf-8?B?QlcxN1lmZnN5dU1ERk16SDV1Y1hqa2FnQXFFVmc2SHgzWGRIL3JRZmcrNTBT?=
 =?utf-8?B?aFJPV2g4ZTF3bzNiUitFZkI5Y3lkZVVNZDZ0WWdmdFhrSk9UZnRza0ZPbEJz?=
 =?utf-8?B?akExWUVNUm1jK2U0VXFIaUJhWDBaYzRGTFh4cnFiMjN0YWI3ZmMzYVhjRlov?=
 =?utf-8?B?RzUzTEFNRFM5R3QvRVNWYUtHREFidmMvQW43OEdNR1NoUW1kcmN2Yldsck8y?=
 =?utf-8?B?eFNzUVozbGxMZHhmWVJoSlNybGhkZWVFbndBdjU5bEFmcnVJZDRtNGxXVmhy?=
 =?utf-8?B?MUY3L0ZUTmMzY0xTVlZ4dHJLVlFpUEQ0SjdONWh1NzZIOHZMN0lRWC9PTDJG?=
 =?utf-8?B?aGx2RHVLQ2Q3VnNEMk5kR1ZlNVFBREJjN3lrVmdwazM5WTRmd1plQjI1NG5s?=
 =?utf-8?B?Q1MyTFFaQnNsUWJXdjFub3RlTlBEaUZkSlV3Y3BhSm1zejBFcXJYd2hlWU5S?=
 =?utf-8?B?Sm8vS0VreWxxT2xyQmpPQkFyOG5QcUtHSDJRKzg1WVl0dXZPU2xadjVzYnoy?=
 =?utf-8?B?K2d6MFoxRURDanRzSmR5YXZTWFk5b2RRUTRYU3JKZXVKOXEzbThmRXdxMXBY?=
 =?utf-8?B?d1JWREJjaFBkekw2R0xaU0JFb2JBN0F3Y1JjZURRVWdrNFVzbnp0L05zaFFl?=
 =?utf-8?B?SjNUNUpzS3RCd1hyYVhpYzYvQWtYT2crcFBGc1kvaWJmTDJrMjZrTXAweHFn?=
 =?utf-8?B?Y3lGNjIydHl2ME1pcTEwaUFEd3U2NlY3VGhhZjZ6TVhCOHRiY1NyZGx1R1FP?=
 =?utf-8?B?bThsUzNkRktOQUpKd1pleTRzbEdENFJRMXV2Y2szd2ZBUHBFMWE3UDZrNGw5?=
 =?utf-8?B?ZWhSd2xLWnJvNHZlRldURi9zeDI5bE9NL2pheDBwYWYyTU9EbGRHUk41NXda?=
 =?utf-8?B?ZWVWNUhzZytzYzR3ZkpYbHdWNzlqNzUrS3VFNEVuY0pIKzhNZ3VLVGh1T25x?=
 =?utf-8?B?YTY3Y1BXaENHbzZUbGNHUnMzRS9QYWxTQzNRbURQLzFsS0ZMZDkvLzY1Y1ln?=
 =?utf-8?B?eGUyN2g3cmhCYTJHR1FiTnVUYkZZUlh4Ym5ldVNTRHVMSmFjNHZzRk05WHA1?=
 =?utf-8?B?R2x3SjhXMFh5OEtKcHc3MENrSjljSWFRaWNxdlNMcWozQUlKekxkR2VtWVZS?=
 =?utf-8?Q?XhyRsPqwArEtAWnsw509J8B58?=
X-OriginatorOrg: suse.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 873354b4-52c2-4fec-83eb-08da8fd3fdd0
X-MS-Exchange-CrossTenant-AuthSource: VE1PR04MB6560.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Sep 2022 06:49:48.6925
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: f7a17af6-1c5c-4a36-aa8b-f5be247aa4ba
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: gET1Fn7mGvp0kv5bOWWgOI7f/Eg22s1VejON6ag5JlNgleCb01PDAcjGZZ/pR8gvfRl/lQHzqY/kbKdClIiZcA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM6PR04MB5957
X-Spam-Status: No, score=-3.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 26.08.2022 20:01, Demi Marie Obenour wrote:
> On Fri, Aug 26, 2022 at 09:53:29AM +0200, Jan Beulich wrote:
>> On 25.08.2022 23:52, Demi Marie Obenour wrote:
>>> @@ -40,6 +41,38 @@
>>>  
>>>  #define efi_data(op)	(op.u.efi_runtime_call)
>>>  
>>> +static_assert(XEN_PAGE_SHIFT == EFI_PAGE_SHIFT,
>>> +              "Mismatch between EFI_PAGE_SHIFT and XEN_PAGE_SHIFT");
>>> +
>>> +bool xen_efi_mem_desc_lookup(u64 phys_addr, efi_memory_desc_t *md)
>>> +{
>>> +	struct xen_platform_op op = {
>>> +		.cmd = XENPF_firmware_info,
>>> +		.u.firmware_info = {
>>> +			.type = XEN_FW_EFI_INFO,
>>> +			.index = XEN_FW_EFI_MEM_INFO,
>>> +			.u.efi_info.mem.addr = phys_addr,
>>> +			.u.efi_info.mem.size = ((u64)-1ULL) - phys_addr,
>>> +		}
>>> +	};
>>> +	union xenpf_efi_info *info = &op.u.firmware_info.u.efi_info;
>>> +	int rc;
>>> +
>>> +	memset(md, 0, sizeof(*md)); /* initialize md even on failure */
>>> +	rc = HYPERVISOR_platform_op(&op);
>>> +	if (rc) {
>>> +		pr_warn("Could not obtain information on address %llu from Xen: "
>>> +			"error %d\n", phys_addr, rc);
>>> +		return false;
>>> +	}
>>> +
>>> +	md->attribute = info->mem.attr;
>>> +	md->type = info->mem.type;
>>> +	md->num_pages = info->mem.size >> XEN_PAGE_SHIFT;
>>> +	md->phys_addr = info->mem.addr;
>>
>> As indicated in reply to your patch changing XEN_FW_EFI_MEM_INFO in
>> the hypervisor: While this may fit the ESRT purpose, the address you
>> return here is not necessarily the start of the region, and hence
>> this function is not a general Xen replacement for the non-Xen
>> function. Therefore I think it also shouldn't give the impression of
>> doing so.
> 
> Is this just a matter of renaming the function?

Besides renaming the function perhaps it also shouldn't give the
impression of being generally usable. I would expect it to be a static
helper somewhere, or even be expanded inline.

>  Is it possible to
> implement the original function with the current hypervisor?

Yes, but doing so would be ugly: You'd need to "bisect" your way to
the start of the region.

As an aside (I think I did point this out before): Can you please
adjust the way your mail program sends mails? When I respond to your
mail (using Thunderbird), I find all the people previously on Cc on
the To: list, while your address is lost. As indicated I believe
this is a result of the Mail-Followup-To: tag your reply came with
(and I further think that TB's treatment of that tag is a reasonable
one, albeit perhaps there are other reasonable treatments as well; I
am not aware of this tag having any formally specified treatment).

Jan