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: <41198c6c-22ad-1530-793a-e557f3c09415@oracle.com>
Date:   Mon, 18 Jul 2022 09:59:15 -0400
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.11.0
Subject: Re: [PATCH] x86/xen: Add support for HVMOP_set_evtchn_upcall_vector
Content-Language: en-US
To:     Andrew Cooper <Andrew.Cooper3@citrix.com>,
        Jane Malalane <Jane.Malalane@citrix.com>,
        LKML <linux-kernel@vger.kernel.org>
Cc:     Juergen Gross <jgross@suse.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        "x86@kernel.org" <x86@kernel.org>,
        "H. Peter Anvin" <hpa@zytor.com>,
        Stefano Stabellini <sstabellini@kernel.org>,
        Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>,
        Maximilian Heyne <mheyne@amazon.de>,
        Jan Beulich <jbeulich@suse.com>,
        Colin Ian King <colin.king@intel.com>,
        "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
References: <20220711152230.17749-1-jane.malalane@citrix.com>
 <272ea76d-0099-873e-b8a8-1cc43b7b1e11@oracle.com>
 <0b5ec408-197c-7e34-28d8-7505e1f031df@citrix.com>
 <01cc9e39-242a-3cbf-13b3-cb3ecccd5737@citrix.com>
 <3be61417-6e58-1d9f-dd3c-7de00daa8d0a@oracle.com>
 <c9aafc28-e93f-ebe4-b591-0edab4efdf2b@citrix.com>
From:   Boris Ostrovsky <boris.ostrovsky@oracle.com>
In-Reply-To: <c9aafc28-e93f-ebe4-b591-0edab4efdf2b@citrix.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?ZndlYnNWei92U2Fyd1pNeitHUDUzNTAxQUhuZmVXNEpIdkJxcmwvemJnSmlM?=
 =?utf-8?B?eFg5dnU0RHByQnB1ZFI3TXBua0tXem1sL0RaTlBJVTNqeGVwMmJtb3g1QW5J?=
 =?utf-8?B?aFZZVGJEVWJmSnlvbmF4V3RrMkttUVZZVmpGQlkxVFY3cjMzWUZGWmtlb1Rm?=
 =?utf-8?B?MG1jdHYwV3FDSjE5dG1kbGFYL3J6QndlRlRZZE5QanlJT0t2YWJndDBFcXFu?=
 =?utf-8?B?UHVnMENTb2RCb3Y3ZUhza2xBbE16ZjB0eXVMUFF1SHZGVlQ1dFBkNUZnaW1T?=
 =?utf-8?B?QlNsZE85Z0dpVXZNVE9NRVNQbnhiS3FsRGN2akJWZnpLbExzNVFhVEYwWHBS?=
 =?utf-8?B?ZGtEdVhzTnRISG50dHZYL2RQMFdhNzRYNGJ0QzZDUnk4aEZ4WHJnd0lkYTJM?=
 =?utf-8?B?MnVZemQ5TkprRmI0N1ZGa21MM0JtQ3JkaHI2eWxRTWVic3FDV2hONDlDQmFD?=
 =?utf-8?B?NUgvQkF5OHp6Umw0Nngwb0hBUG5wcFlJUHpBUjFxRXRjZGhXYlBlS0lROHVx?=
 =?utf-8?B?YTdtc0lsSmZtNDhwVC81VVN3VFZMb0lYSkJISnM4eXMvV2pNRnoxZStIQnNz?=
 =?utf-8?B?MUlYZkE2T01TNzVZSW5oa3ErSmxFaXpVcXZUSWtReElCakxxMVZQSmxvbldo?=
 =?utf-8?B?cWpvVkRJQjQ1OVREdWFDUnJhNEg1R2JYSFZMcS9CS0JzaWF1Tk9vSG83RDlk?=
 =?utf-8?B?Z1BmQVFHa1hLTjJYNVI3OWxuUEU1MDJBNGRkOFBTTERhZHRYMUtEM0FlamZy?=
 =?utf-8?B?dEl4cXFzVVUwOXBLL1hEQm83c0tWeHdOWWFsSWRWeVh1WnJLWlJDdUFpYWtT?=
 =?utf-8?B?Yit4aS9Hb09yYjdINWNEbVNVNmRWckd6UXpoZ0ZzRjYvL255NUR6MFFvSFI4?=
 =?utf-8?B?T3o3UGRxcHd2RFgvaG1rMktPYXAxZFgrVE1HaTRtMHFzQUhFazMxazBpRjZF?=
 =?utf-8?B?WXBxUmI1R2ZMOW5pa0YrM0VEa0JNZllSemxwOFNCd0I4SkNNQlVzdmJUOEMr?=
 =?utf-8?B?OE4vSC9Vd0hHWnN6NzVSN3ovQVZ2Zm9PeGxacGFMWGl6SXBoOHU5Q1VWT3lW?=
 =?utf-8?B?MG1zSkJoUlFpYjRFS0xFYStDK3pucWRTTmhaelYwV01qdmFLL2N5YlZ2NUp5?=
 =?utf-8?B?aDJxczY0V1RkTkNZRFNuQVVlTE5IU2E1LzJaMTFVNzZZbnRRY2dWbno3TUhz?=
 =?utf-8?B?Z3NOZm1vb0o2QWZoWi9IMkRMRFJWcDVZRTM0ektOU1g5bDI2SjMyRjA3czh3?=
 =?utf-8?B?NVFjbzhwMnVaSnRnMGtWOGVFckY1SXRIUDhVaGNjanNnZlcreHBrYzFFZTZv?=
 =?utf-8?B?akg4Uy85dEwwUlBScnFoMHJMQ2IwcFJ5WE5EZ0ZmMEtyL3JNY25PNElqVmVB?=
 =?utf-8?B?cU5BYXpuT3p2NmQxd053a205bElreTNINy9senBXdytYekNPZFhyMEZDKzBC?=
 =?utf-8?B?ZDFhWWttYlE2U1RLWCtpK0MzVlFzcEpjK2xtdmxZQUVzbnlkTlJzVURQc0Zu?=
 =?utf-8?B?c1M5VGRpRWt2eU5EL2lWQlhGNmhva2hYMkw0aGdpd3ZZUEppRmlLTzluOXZ2?=
 =?utf-8?B?Njd2RmFRMnRQRkRNbVdhbThsdUx6SkVEamtPOEg0VjQzdmc1SUpNanB4RW5U?=
 =?utf-8?B?T2NBazh4N2paTDl0bU5BdnBOcE1kay9qZC9EY0xCR1htNEtXVGFkdzFwNFdu?=
 =?utf-8?B?bFIzN01FbzQyZmdlVGhKOHJEVnlTOTJBd3lTWnRqUENBUG5QRzVVWmg0R1cx?=
 =?utf-8?B?NkdiakJVNzYwS3k1T1hrLzBvQWhBZ0ZnVzE4d2RlQllxVmZDNGIrRjFSTWNE?=
 =?utf-8?B?L3hCaUl1ZW5NbkR3N0tXS2tBVUs3SjZGZ28yaGZyanA4cS85aEtVRXVjd3FR?=
 =?utf-8?B?MWZLK3FLQ0tweXUvTkwyQ0lkVVBHcDVzcG4xb3BHSnAxK21HdkZTWGFWb2ln?=
 =?utf-8?B?Q3I1OGl5RVNXcjlVVFl5OGJQWUt0L3MrcC92V25XVVUzZEVSUDFWNG43WXJD?=
 =?utf-8?B?YmZOc2pkaFROU3BkcXFBSDZGTWEyL3BxY3d3ZkVNYWpmY1VndkVlNkE3ZERw?=
 =?utf-8?B?UFVCZlp3S1VnQndlRUE1WXV5dUQ0aUZPd0RLTHFxS1pYcjFkTnBKbnI4Z0dY?=
 =?utf-8?B?dVJFNWxxT2pIaEZlRm9INlEzODRqUlo3QlEyREMvOU41a0JiUVFnaEt1VmlJ?=
 =?utf-8?B?bEE9PQ==?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c3a09814-81bb-4e28-0b3b-08da68c5b72a
X-MS-Exchange-CrossTenant-AuthSource: BLAPR10MB5009.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 18 Jul 2022 13:59:21.9990
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: WMnXgP8734lLmZuLl56Yc7mZJ2n8T1SDFvx5pODsaHy6k4/fFQn6bbuyOqGWOShAAyGURRrlmxjWTFmr49/wZCYhZ3gpoWGK0+B5k6kOsow=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR10MB1870
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.883,Hydra:6.0.517,FMLib:17.11.122.1
 definitions=2022-07-18_13,2022-07-18_01,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 suspectscore=0
 mlxlogscore=999 malwarescore=0 adultscore=0 mlxscore=0 spamscore=0
 phishscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2206140000 definitions=main-2207180061
X-Proofpoint-GUID: Vd43k06TR7kQN9IXsX-p8kkE8x_saDG1
X-Proofpoint-ORIG-GUID: Vd43k06TR7kQN9IXsX-p8kkE8x_saDG1
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE 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 7/18/22 4:56 AM, Andrew Cooper wrote:
> On 15/07/2022 14:10, Boris Ostrovsky wrote:
>> On 7/15/22 5:50 AM, Andrew Cooper wrote:
>>> On 15/07/2022 09:18, Jane Malalane wrote:
>>>> On 14/07/2022 00:27, Boris Ostrovsky wrote:
>>>>>>         xen_hvm_smp_init();
>>>>>>         WARN_ON(xen_cpuhp_setup(xen_cpu_up_prepare_hvm,
>>>>>> xen_cpu_dead_hvm));
>>>>>> diff --git a/arch/x86/xen/suspend_hvm.c b/arch/x86/xen/suspend_hvm.c
>>>>>> index 9d548b0c772f..be66e027ef28 100644
>>>>>> --- a/arch/x86/xen/suspend_hvm.c
>>>>>> +++ b/arch/x86/xen/suspend_hvm.c
>>>>>> @@ -5,6 +5,7 @@
>>>>>>     #include <xen/hvm.h>
>>>>>>     #include <xen/features.h>
>>>>>>     #include <xen/interface/features.h>
>>>>>> +#include <xen/events.h>
>>>>>>     #include "xen-ops.h"
>>>>>> @@ -14,6 +15,23 @@ void xen_hvm_post_suspend(int suspend_cancelled)
>>>>>>             xen_hvm_init_shared_info();
>>>>>>             xen_vcpu_restore();
>>>>>>         }
>>>>>> -    xen_setup_callback_vector();
>>>>>> +    if (xen_ack_upcall) {
>>>>>> +        unsigned int cpu;
>>>>>> +
>>>>>> +        for_each_online_cpu(cpu) {
>>>>>> +            xen_hvm_evtchn_upcall_vector_t op = {
>>>>>> +                    .vector = HYPERVISOR_CALLBACK_VECTOR,
>>>>>> +                    .vcpu = per_cpu(xen_vcpu_id, cpu),
>>>>>> +            };
>>>>>> +
>>>>>> +            BUG_ON(HYPERVISOR_hvm_op(HVMOP_set_evtchn_upcall_vector,
>>>>>> +                         &op));
>>>>>> +            /* Trick toolstack to think we are enlightened. */
>>>>>> +            if (!cpu)
>>>>>> +                BUG_ON(xen_set_callback_via(1));
>>>>> What are you trying to make the toolstack aware of? That we have *a*
>>>>> callback (either global or percpu)?
>>>> Yes, specifically for the check in libxl__domain_pvcontrol_available.
>>> And others.
>>>
>>> This is all a giant bodge, but basically a lot of tooling uses the
>>> non-zero-ness of the CALLBACK_VIA param to determine whether the VM has
>>> Xen-aware drivers loaded or not.
>>>
>>> The value 1 is a CALLBACK_VIA value which encodes GSI 1, and the only
>>> reason this doesn't explode everywhere is because the
>>> evtchn_upcall_vector registration takes priority over GSI delivery.
>>>
>>> This is decades of tech debt piled on top of tech debt.
>>
>> Feels like it (setting the callback parameter) is something that the
>> hypervisor should do --- no need to expose guests to this.
> Sensible or not, it is the ABI.
>
> Linux still needs to work (nicely) with older Xen's in the world, and we
> can't just retrofit a change in the hypervisor which says "btw, this ABI
> we've just changed now has a side effect of modifying a field that you
> also logically own".


The hypercall has been around for a while so I understand ABI concerns there but XEN_HVM_CPUID_UPCALL_VECTOR was introduced only a month ago. Why not tie presence of this bit to no longer having to explicitly set the callback field?


-boris