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: <29c65421-2478-4e29-d803-2dbecc9ffe1c@amd.com>
Date:   Tue, 22 Nov 2022 13:17:31 -0600
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.2.1
Subject: Re: [RFC PATCH V2 16/18] x86/sev: Initialize #HV doorbell and handle
 interrupt requests
Content-Language: en-US
To:     Tianyu Lan <ltykernel@gmail.com>, luto@kernel.org,
        tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
        dave.hansen@linux.intel.com, x86@kernel.org, hpa@zytor.com,
        seanjc@google.com, pbonzini@redhat.com, jgross@suse.com,
        tiala@microsoft.com, kirill@shutemov.name,
        jiangshan.ljs@antgroup.com, peterz@infradead.org,
        srutherford@google.com, akpm@linux-foundation.org,
        anshuman.khandual@arm.com, pawan.kumar.gupta@linux.intel.com,
        adrian.hunter@intel.com, daniel.sneddon@linux.intel.com,
        alexander.shishkin@linux.intel.com, sandipan.das@amd.com,
        ray.huang@amd.com, brijesh.singh@amd.com, michael.roth@amd.com,
        thomas.lendacky@amd.com, venu.busireddy@oracle.com,
        sterritt@google.com, tony.luck@intel.com, samitolvanen@google.com,
        fenghua.yu@intel.com
Cc:     linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
        linux-hyperv@vger.kernel.org, linux-arch@vger.kernel.org
References: <20221119034633.1728632-1-ltykernel@gmail.com>
 <20221119034633.1728632-17-ltykernel@gmail.com>
 <116799e9-8b14-66d6-d494-66272faec9e9@amd.com>
 <2f3c100f-355d-e4f2-ff42-2cb076e8aa86@gmail.com>
From:   "Kalra, Ashish" <ashish.kalra@amd.com>
In-Reply-To: <2f3c100f-355d-e4f2-ff42-2cb076e8aa86@gmail.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?WXhIL2ZtU3M5ZGZiNWdnS1BpblFDdjU5RWhZc216S3JqNGcwL1VCQlBMWXlX?=
 =?utf-8?B?cnF1MkFrRFVIUE13dC9zVFFHaVR1MGdSa0ZFODZvcnR5YWFsTDRWNitUaUhP?=
 =?utf-8?B?bVRTcnpabXJCZjBVOXBuVUp1SlBjT2xNbnBlMDRSYW9mcnNxUUxxWmxSQ1NQ?=
 =?utf-8?B?WGZTTllzYzFZR2pqTnFGM25FWmpJRGJINzhRWUp3dy9IWkE4MFJpQ0ZLVEpx?=
 =?utf-8?B?cEpJYVMvakt5bmdKbHlFMFYzNGRMWDJXKzc2ZitsR3lCRXRPMVR6ZjhuT2RP?=
 =?utf-8?B?ZkNzTjhaRVBSaVJEWG1BK1hNUUJjNEs4QzJtb1pQcXZIUGpFenlGQ2pLYjU3?=
 =?utf-8?B?c01ZK0NjQ3FzdUN3RHB5dldmWEpGcGJLZDJwVUIwYjNSaVNTYTJEdjBVU0Nx?=
 =?utf-8?B?cjNOVnVBWFl3SytlTUlnVmJLWitPRllNWHBjdVErdnJoQ2RrYTY3b2tTVzBh?=
 =?utf-8?B?UEFJZ3N4U0NkNTN3ZGIrNkh3YUtOdXdNUVlDbDVqMEVlbVhiRmpRWXFGTG9y?=
 =?utf-8?B?UjAzdlhLNTZLczczRlVpSmltSHQrTEhrM0pxemtSL0JnRlg5dzFVQjVRejVl?=
 =?utf-8?B?Q2ZWZHRRdFRtZ3hXeERDVy96dUhGWVBFYTlVamlPYnV0VnAwc1I0MXZYd2lV?=
 =?utf-8?B?TTI1WXh3Z25ERHZYTlNOK2JBYWRidjI4Tk8yYWtISENEZ21mcGh3b2ZwQUlq?=
 =?utf-8?B?ZjdjYkM2Qk4rQUdHeVNZOXlkNHpUeDV0QkNGVG5GcFVKMHlhSWdxVzhicmZS?=
 =?utf-8?B?OFVUWWY4RXRZbW9TaHYzelRRbmh3ZHhjQmlkOUt0ZEh0MThPTWFzVTNVME1Y?=
 =?utf-8?B?dXl4d0RaK004bFEwZjBHcU5FRDNmSTE4R3kyUWlqSUwvWDY0bnUyNGR6Y0dj?=
 =?utf-8?B?aUlLSm4ycXZlbkpvQzc5SUZsV3Q4QjhITVpaWHVhZUNBMkNucHZUN2UyY3h4?=
 =?utf-8?B?SzA1U3ltRlNSUWpzbGF4WGhvRDF1RUE5QzVaZE9XLzZqOC95MnZ5OXR6OVpR?=
 =?utf-8?B?OHNhWUx3V0pCVUpZSlVKcFhtQ0tTcVJUL0F4Q1MrS0R5eEJRdmg3b0FzdUl2?=
 =?utf-8?B?UTJxaVJuc28yT2wyRDhaU1kvQS8vQTBsRW5BdlQrL05pSVJCUDIzYmFOWGU4?=
 =?utf-8?B?NDlyUmZRbDhZcVRnckNIWjE0cXdiZXQ5RHhtaStCUTlZeEhHOUtrN3JSYTMr?=
 =?utf-8?B?Zy9MS2FpNU54VVBQQmhIbW1lLy9CZE5CSGI4VjlQdXVOdWdmbmp6RU1TQlZE?=
 =?utf-8?B?aEx1bUVZVTd6Q2wvVHMrVUVzM25VVEVQdWpYUERybUdKdGE5UThpTHMwNmxr?=
 =?utf-8?B?VnE0NVdhNDBnUEZXZGw2NEMxSlhTZGdZREsxSHk1YUdSRXd4MzZadzAzWGdu?=
 =?utf-8?B?N0hUdUZGTUM5WCtabFYrVHFwRXV5clpGZWozRXRidTlWdS9HMmhXUDVJc1Np?=
 =?utf-8?B?OUY5L3cvNmlkejhuaFY2eHVpTzIwK2gwU0pYOVBmbG9vVFhWdTNlMy92VHJI?=
 =?utf-8?B?TUJlZ3dSdFZFK1lsdUx1WWQ0RDdBeDQ5L09qSTdYTmdBRFhaZ0UvQlFYalFZ?=
 =?utf-8?B?QWVoZnhBTStkSkllMEtobTBrVDdKQUZsVGdLdk5zTmFGSUVRRTI0OHJtVGlV?=
 =?utf-8?B?UXQ2dXRwYmhQRDF0V1VvMWY2TStJaytRRERqVWhiUWJUVHhVcm5wc2ZJRHo2?=
 =?utf-8?B?dDRTMHQ2alBDWUlNWUc1UXVYV1k5NDQ5UGJMZFd0MUZ4bXNNaGlLSTI1MXJ0?=
 =?utf-8?B?YlNkVUVtejh3c2ppdU13R1NvQ2tHdTRTcHkvempRekwzWXQ2Zm5MSS9SQWZn?=
 =?utf-8?B?KzNseVFCeVhnR1VFazlmTXR2TXJ2SnVjc05naDZIZzlXbmxIUXNtWTBuSFor?=
 =?utf-8?B?S1lKaVpuZEd6UFUrN2RSS3ZvZ3hBYVdDQS92SXhtVnJhQ0JLeVptRDZSSTg4?=
 =?utf-8?B?Uk5SWSt1d0xEekg3eUUrWWFUTVY3OXRSd29MeUhua1VSODdoTDFhSlJDL3RD?=
 =?utf-8?B?YjByVzBOeHhMZ2Qwc1ZCekZ6UWVoa0JxOThSaGdnTGlINHdScFVtWE9NZlFs?=
 =?utf-8?B?MkJDa2JndzVVRHRZWHVsZkpucWVzYUVRdFFUTDl3LzdHNFlMVkYxd0s4SERV?=
 =?utf-8?Q?RK3GI/Jofddof15fhxuVYj5cW?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: fc354a68-1be9-41f9-df68-08daccbe36d1
X-MS-Exchange-CrossTenant-AuthSource: SN6PR12MB2767.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Nov 2022 19:17:36.4422
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: X2oAxX6qnRH8lnz9ZbwaEvJA/fL5v+VJYqDaxoX5SqCywmBhxCzT3U+sWTj6FB5apdKvjTn89ZzCWCltJ0Q6/g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR12MB7503
X-Spam-Status: No, score=-2.1 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 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 11/22/2022 7:46 AM, Tianyu Lan wrote:
> On 11/21/2022 11:05 PM, Kalra, Ashish wrote:
>>> +static void do_exc_hv(struct pt_regs *regs)
>>> +{
>>> +    union hv_pending_events pending_events;
>>> +    u8 vector;
>>> +
>>> +    while (sev_hv_pending()) {
>>> +        asm volatile("cli" : : : "memory");
>>> +
>>
>> Do we really need to disable interrupts here, #HV exception will be 
>> dispatched via an interrupt gate in the IDT, so interrupts should be 
>> implicitly disabled, right ?
>>>    panic("Unexpected vector %d\n", vector);
>>> +                unreachable();
>>> +            }
>>> +        } else {
>>> +            common_interrupt(regs, pending_events.vector);
>>> +        }
>>> +
>>> +        asm volatile("sti" : : : "memory");
>>
>> Again, why do we need to re-enable interrupts here (in this loop), 
>> interrupts will get re-enabled in the irqentry_exit() code path ?
> 
> Hi Ashish:
>      Thanks for your review.    check_hv_pending() is also called in the
> native_irq_enable() to handle some pending interrupt requests after re
> -enabling interrupt. For such case, disables irq when handle exception
> or interrupt event.
> 

Then probably add the interrupt disable/enable in the caller
function like native_irq_enable() which seems logical as 
check_hv_pending() is always called with interrupts disabled (either via 
HW or SW) and also interrupt disable/enable seems redundant in 
check_hv_pending().

Thanks,
Ashish