Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH] xen/x86: fix PV trap handling on secondary processors
To:     Juergen Gross <jgross@suse.com>, Jan Beulich <jbeulich@suse.com>
Cc:     Stefano Stabellini <sstabellini@kernel.org>,
        lkml <linux-kernel@vger.kernel.org>,
        "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
References: <34898e9c-5883-a978-98ee-b81b22d8caed@suse.com>
 <823f4ef4-f9e5-68cb-d6e9-d079483c1e21@oracle.com>
 <0afce6e8-3c8a-e5ae-cd54-0fd598276506@suse.com>
 <62ccf5b7-b903-e604-d113-67c7633278cd@suse.com>
 <2c4549c8-bdeb-3584-95c4-7076b7cf79bb@suse.com>
 <35835650-bbbb-7dbd-061b-693f39f9453e@suse.com>
From:   Boris Ostrovsky <boris.ostrovsky@oracle.com>
Message-ID: <1bdda8a5-4748-6791-58a9-b1e342281b01@oracle.com>
Date:   Fri, 17 Sep 2021 13:50:54 -0400
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0)
 Gecko/20100101 Thunderbird/78.14.0
In-Reply-To: <35835650-bbbb-7dbd-061b-693f39f9453e@suse.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
Content-Language: en-US
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?dGtEbXZDanhHUTlFWHBwYktiN3hRUkZ1a1VHWUVnM2djVjF3ZEZoRVMyNDlV?=
 =?utf-8?B?a1JpcTYrd2wwajRDMEUzY2xZeXN5N011R2NoL3JBRXNVMkNGYnBmN2c4YXIx?=
 =?utf-8?B?WitmSGFVK0pLZ2NkN1p2Z2JNWVQzbzg0ZGpVYVd1MlA1TmhoWVN5L1B2U1J2?=
 =?utf-8?B?eUVPSUl0S3NRbmNwckhTcmRjd0VxVVdEVXprZnVHaUxEUDc4RnJkeUkwaGlt?=
 =?utf-8?B?ZmJFQVJlK2JEMlpvSkxiM2Fzd2dHemt0VHRTMzFTRU4rTU5JcHQrNTdvMzA4?=
 =?utf-8?B?NENidjN5eFBZVWxtenBkWk9Rc1B3RU1DTm1ocDhUSGNQUWhWSWpVNFZybkVv?=
 =?utf-8?B?aS80aW5zMVNkRDBGT2FtLzk3L0Zsa2RHSnVEQXNxWnZDWkVmUEtReUFMd3RU?=
 =?utf-8?B?ZWVkbXZHTFgvY05xT254M2NPQlBVSmNYa0czcnBiSEJPNlNjY2w3R1JlOVd6?=
 =?utf-8?B?N2lyVVBPQzIxS1pwUGdNTGpGc2R2VFZVOUlxUko4SkF4N1I0eDRPdkdOYWtG?=
 =?utf-8?B?bG9vSmlyc1Z1MFJucmV6ZnViWEcvb0NoUGxVT0k1MEMxcTE1UVd4MWQyQ0Nt?=
 =?utf-8?B?clFtdkdhMXN4TGtJUVJjZlFodjhCa05USVl5Q2N5ZjF1c2prSlJtc1FPSFY0?=
 =?utf-8?B?bUJYZ2x0bG5ZdVVES2dVbFRwVTN0NmRKMFBaTEZTTHp3NFZHVnBlVjlPS1Zr?=
 =?utf-8?B?WStzVWszRHFVMVRJdXl4bGxZOFVSblVhREtMUkxOckJ0SGl0TjlpN0cwaXpV?=
 =?utf-8?B?SU45eld0aXA1L3lRYmRMcjNLdDRldEpUT2o1ZGhXMitZOUVsK0lMMFAvNVBl?=
 =?utf-8?B?U3l4SytzaDNxaDQwaHFQejFpMHVXMkRSSGlJenk3TjFBODFiSVhvVWFwUXhS?=
 =?utf-8?B?QUM4NTIzUHIrenNuZWcvQW5MZHoyYWFGTm11MDUyZXh4V1lJdVppRGFrajVT?=
 =?utf-8?B?MFBFL21GYzFwOHQ3RUo3SHIxRGRUd01VVUtFdFB0Sk9USWUyVUpsZnh5dFp0?=
 =?utf-8?B?S0tYQmh2Qm5qakE5V1lpS0FGSGIxa2JTc2FhNytINWZ2ZkhnallGVHVtUkta?=
 =?utf-8?B?M0k3VnBZWTd4M0p2dk9KVGM5anVqSW1KdTUzaTgzSUFxbjhxVGxWZVJablAz?=
 =?utf-8?B?MzV3aEFqcElJM3RHVE5EY1BUaGlxSkU0YkhSdC95Y0cybkpoSXp0RW1OZnZu?=
 =?utf-8?B?SnU1SXA0bnpnZ3k3YldGNHYzY2tWK0tRVkpjWnNnQmZlU0ZLVGZzZytHS1dn?=
 =?utf-8?B?bUdnb0d3L1pFTlBrRHZTdFVRY0J3M2d6ejVDaU9QMWU4WktsdlZoa1FsNTdQ?=
 =?utf-8?B?WG5zMU94WWxqeGFHbENKdU4wU1Z5TGtwVmZjYmxIdGhUa1NNejFrRDd3R1hS?=
 =?utf-8?B?VVRHR1VZS3gxZ1p4bVllSkdnK3U1cGlLbllVVHJYS3VNOW5OUXlwMjFjbGRo?=
 =?utf-8?B?b0dVT29ZeFc4ajg5ZkUvczFwcFB4b2k5R3FiRENCejB3NDJUd2N0SVhOdWwz?=
 =?utf-8?B?ZEJXdW1SdDNvaWM5dU1IWWkvSlZibnFoc3M3WERnNkNJSmFQQUZDYzFGbkhW?=
 =?utf-8?B?ZUhpK0YvYnpVbzJ0aGQ5cm1NYk96SFZubzBsQlhwTHBhcEtDbnJjSHExcDh6?=
 =?utf-8?B?MXRMK2ZndUh5cnBvb01WeGx5WmZyZjRTeVYxZVdHRVZVNFpOTjNSR3Nnb1ZO?=
 =?utf-8?B?UE1EdzBaZHFQQk50YzlXcDV0NHJWbk1hcXErZ2l1Q0RqcDVFc2NQTXE3MVFC?=
 =?utf-8?Q?HzJGQwmsTUN02aqc/gM/dHEKTotHCYgrQF/HDr2?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 20811149-4c5f-4209-bb56-08d97a03b395
X-MS-Exchange-CrossTenant-AuthSource: BLAPR10MB5009.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Sep 2021 17:50:56.7584
 (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: 1M7lCaYKdya8lcRjxAFU/MwWD+sIZw9IBG3tBSWqycq9JoSh4gknGQZ86xLw3IUBuHM8VEnqnBSNJ/5oOyUCkz7/KhFjeaTVpZIh1juUk6A=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR10MB4318
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10110 signatures=668682
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 phishscore=0 malwarescore=0
 adultscore=0 mlxscore=0 mlxlogscore=999 suspectscore=0 bulkscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2109030001
 definitions=main-2109170108
X-Proofpoint-GUID: qz4rghgpVjQRIrD0YimMuXXzC1OH82TJ
X-Proofpoint-ORIG-GUID: qz4rghgpVjQRIrD0YimMuXXzC1OH82TJ
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 9/17/21 3:24 AM, Juergen Gross wrote:
> On 17.09.21 08:50, Jan Beulich wrote:
>> On 17.09.2021 08:47, Juergen Gross wrote:
>>> On 17.09.21 08:40, Jan Beulich wrote:
>>>> On 17.09.2021 03:34, Boris Ostrovsky wrote:
>>>>>
>>>>> On 9/16/21 11:04 AM, Jan Beulich wrote:
>>>>>>    {
>>>>>>        const struct desc_ptr *desc = this_cpu_ptr(&idt_desc);
>>>>>> +    unsigned i, count = (desc->size + 1) / sizeof(gate_desc);
>>>>>>    -    xen_convert_trap_info(desc, traps);
>>>>>
>>>>>
>>>>> Can you instead add a boolean parameter to xen_convert_trap_info() to indicate whether to skip empty entries? That will avoid (almost) duplicating the code.
>>>>
>>>> I can, sure, but I specifically didn't, as the result is going to be less
>>>> readable imo. Instead I was considering to fold xen_convert_trap_info()
>>>> into its only remaining caller. Yet if you're convinced adding the
>>>> parameter is the way to do, I will go that route. But please confirm.


Yes, that would be my preference. No preference on where to set the sentinel.


Thanks.

-boris


>>>
>>> I don't think the result will be very hard to read. All you need is the
>>> new parameter and extending the if statement in xen_convert_trap_info()
>>> to increment out always if no entry is to be skipped.
>>
>> And skip writing the sentinel.
>
> Maybe it would be even better then to let xen_convert_trap_info() return
> the number of entries written and to write the sentinel in
> xen_load_idt() instead, as this is the only place where it is needed.
>
>
> Juergen