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: <ea76eda5-2736-aacc-5256-2bee7b9247fa@oracle.com>
Date:   Mon, 25 Jul 2022 16:46:53 -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:     Jane Malalane <Jane.Malalane@citrix.com>,
        Andrew Cooper <Andrew.Cooper3@citrix.com>,
        LKML <linux-kernel@vger.kernel.org>,
        Xen-devel <xen-devel@lists.xenproject.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>
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>
 <41198c6c-22ad-1530-793a-e557f3c09415@oracle.com>
 <b0124d4d-32c9-01d3-36e7-9890ea990fdf@citrix.com>
From:   Boris Ostrovsky <boris.ostrovsky@oracle.com>
In-Reply-To: <b0124d4d-32c9-01d3-36e7-9890ea990fdf@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?L2hURnNCZkRzK2RpR081SHpMTlcwTjFQWGNlK1RDUi9YYXNEVG01cFBlaStM?=
 =?utf-8?B?U2RIcVlJS2ZIQnJnNkdxdFdHNllic083Ui8rZHlhd0dLZ0dJcU54YzdzQm5p?=
 =?utf-8?B?K0NDWWd4Rm1mam5MV1A1b0gxajVpTmVXQVJvVkxvazFiZU5mdGVYVFE4Tklt?=
 =?utf-8?B?U1Q2RlFIYm9za3pqRHd2bytRdWE4a1JvL1E4YTE5aGpDVlplamlhRm5uUDJ0?=
 =?utf-8?B?NHJndFhvdUlFS2pTNkxTV2k0RktrQ2t5U0FYdDZtS2k3R205TEpFMnM5YVZT?=
 =?utf-8?B?MUdpOXQwbWtnMkZCdGJZR2U3NnpJM0U5Z3dabzdnc2gvSldQWE5kWkdyUzA3?=
 =?utf-8?B?SmZNNS8yalprcnR1ZUMweWRuekhPV3RSRGRMdlFYaXZTOHVVWk1ZeWRaK1E1?=
 =?utf-8?B?ajJQQUkxZzdGUFRPZGRROFB4blBxUm5rd1k3aUlxUHQveTYxOEo1YlJFRlBO?=
 =?utf-8?B?a0dnU21FRmFoSVpYMk1LcVovR2VUR2RvT05xS1RWd04wbE1YSDhsckNuZ29D?=
 =?utf-8?B?ZlJUcEc1Vm1pWThBcnRWVjY5Y3VvVmNUUHhJRS9VNWZyV0ZwSFZKTEIrOFlK?=
 =?utf-8?B?T1ROeEpqYyszckEzdyt3T2VIdnlkSGR5UU1LeUk5Ny9EN0lXYWF5ZmMxQXVn?=
 =?utf-8?B?RlJUSlIwamNCMi9tNndBbEI5MFY1N0dOMHpabENpZ3k5RnUveXNUTVZGVk5p?=
 =?utf-8?B?VXR0aSt0VTV4dGNQb1Q4ektsVTZqaW5LYUxDUE83Z1NYdWgrdGRjTUwvR1p5?=
 =?utf-8?B?Zms4MWEycmNueDgvQ28rMjJTVHpxNjBGZEpPVm1sNmJPajFwdTZubjVRUnVq?=
 =?utf-8?B?T25RcmxqWmFmdVB3d1k5V0c0SVpQS0NleThWL0RZeXhxMU81U282cDRVU1Av?=
 =?utf-8?B?L3RScWwzcW5JcWF3d0c3Yzd4QlE3MEtIbUVXWlpXcU1ScnphRnNyc3h5SEtv?=
 =?utf-8?B?bjVvekU1VnQ2SlpSczFTWlFTc1BVTTY2RlZFQUZNeTlJbVRpU25WM0tuY0Ja?=
 =?utf-8?B?TjQ1N2ZCSmNCNE9UV1FTQzQ0SXJDb28xV0ExTU1hWXpSbXRYdmVpSmFxbFRI?=
 =?utf-8?B?VVdxblB6Vm9kYkg1M3VkSTVUWXJIRnJCbnFvTE40MzhFSTl3a3JhREt3TXA2?=
 =?utf-8?B?TENaUkhRanM5bFNmOFY0MXlqZncweEpQbzlKL1RHVCt5VFE3c2xSbkNOTDM3?=
 =?utf-8?B?SGlpbHRGa0RYVUV5Qkw0ak1HSEhWMU5nd20wLy9uaFZ4K2ZHdzdubUM1QjNx?=
 =?utf-8?B?eGRGQ0FnRkxqTUNqWG8zV2xiUnVKbjc5YXdKNmQxZDFmYlJvL1lBNkh5YzlH?=
 =?utf-8?B?Wk1lek93YUdXaVo0MzgzMVVqaXhCMXFxMlk3MTJDTVRBa0M5MC8ydXZESi8w?=
 =?utf-8?B?c0htUjFYeHhUNHNpNnpvaTg1L1FsenhOdEFRWE8yR2g5cVYwTVcvQ3FSVVYy?=
 =?utf-8?B?NUdNSlVHY25RZG1vSVkvU2xETnpEY011MnJpNkgxUE1leFJ4NFM0M0NvQ1pw?=
 =?utf-8?B?aWlIdTNucXN2WTVHcWxXdXA3L1JBVnRtVE5Sd0FHb0hBSjNHZU1LdFNjODc2?=
 =?utf-8?B?THh5M1o4RUZSS09EdWNHRnAxRHdwT2J0OU9xNUdBUmZ4aUhyK2FSY3U0dDd4?=
 =?utf-8?B?eTVBY1ZKdk5ETUQ5bGlXMW82dmVSTytTTFRvU0RxTzlKYlBzNmNmVHpQamVn?=
 =?utf-8?B?Q2hZdlEvTlBDZmovREFGOHlYbUs2K2haMXF1cko2VTFpQ0hOaEh2TTlwb3JJ?=
 =?utf-8?B?WUsxdmNzRkdROTM3aHFkLys3anBpWTVpWk90d0VBb2VxVnZHbjZuWGlTMEZ1?=
 =?utf-8?B?cE1MRFdPb1N6RnpBYldYcy9raEJ6VjErWFhuOVJOTzg4bndBcGJzVnlTblg0?=
 =?utf-8?B?NExPS21FQllqTG9KRDBtT3QvMURZeEYxQjJiWk5XR3NUUHlxWmt6b0xCVisz?=
 =?utf-8?B?dmpqNDJQcDl0b3gxbVI4N3FTbDNYdG04MmdENHgwNlBYK2NtUGFUdm83Z2pB?=
 =?utf-8?B?MzZnTXlHcGRMSWVXbmZtc3VtUk9KSXJRbnZ2M00waHluZnJuTWNscHVvR24y?=
 =?utf-8?B?QUZnWkpoZCtXL29QSXEvM3NJSm5JQW92TStIaGVZTzc5WXlVQURHMU1OVjd2?=
 =?utf-8?B?UEJ5RkxkK3RvMjVWb2g5V2poTmRSK1Zkc2xaRGNWOHN6Yllsb3V4T0gvNjBY?=
 =?utf-8?B?RlE9PQ==?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 708a937b-6ec0-402c-a09d-08da6e7ed1ac
X-MS-Exchange-CrossTenant-AuthSource: BLAPR10MB5009.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Jul 2022 20:46:59.1063
 (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: FvzXvS+O9MT8EPHDpMSl5FioSDsqkawD07DWiCu+AF2IUOv/6Al3H/B7rriA6qfV8897jMxOrK0FUCHQGcu2WywMOx4cvqaaEtuvt7z+6RE=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BLAPR10MB5283
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-25_13,2022-07-25_03,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 phishscore=0 bulkscore=0
 adultscore=0 mlxscore=0 mlxlogscore=999 spamscore=0 suspectscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2206140000
 definitions=main-2207250084
X-Proofpoint-ORIG-GUID: hKu8nYe2Keo72yhHldu1TYhEa_9LyMuG
X-Proofpoint-GUID: hKu8nYe2Keo72yhHldu1TYhEa_9LyMuG
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/25/22 6:03 AM, Jane Malalane wrote:
> On 18/07/2022 14:59, Boris Ostrovsky wrote:
>> 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?
>>
> Any other opinions on this?
>
> (i.e., calling xen_set_callback_via(1) after
> HVMOP_set_evtchn_upcall_vector OR not exposing this to guests and
> instead having Xen call this function (in hvmop_set_evtchn_upcall_vector
> maybe) and tieing its presense to XEN_HVM_CPUID_UPCALL_VECTOR which was
> recently added)


CPUID won't help here, I wasn't thinking clearly.


Can we wrap the HVMOP_set_evtchn_upcall_vector hypercall in a function that will decide whether or not to also do xen_set_callback_via(1)?


-boris