Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757666AbcLTKbQ (ORCPT ); Tue, 20 Dec 2016 05:31:16 -0500 Received: from foss.arm.com ([217.140.101.70]:39092 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757567AbcLTKbM (ORCPT ); Tue, 20 Dec 2016 05:31:12 -0500 Subject: Re: [PATCH] Xen: ARM: Zero reserved fields of xatp before making hypervisor call To: Jiandi An , Stefano Stabellini References: <1482116198-28940-1-git-send-email-anjiandi@codeaurora.org> <5858BB54.5090106@codeaurora.org> Cc: Juergen Gross , boris.ostrovsky@oracle.com, xen-devel@lists.xenproject.org, linux-kernel@vger.kernel.org, shankerd@codeaurora.org, shannon.zhao@linaro.org From: Julien Grall Message-ID: Date: Tue, 20 Dec 2016 11:31:07 +0100 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: <5858BB54.5090106@codeaurora.org> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1335 Lines: 25 Hi Jiandi, Please respect the netiquette and wrap line to 70-75 characters. On 20/12/2016 06:02, Jiandi An wrote: > On 12/19/16 12:49, Stefano Stabellini wrote: >> On Mon, 19 Dec 2016, Juergen Gross wrote: >>> On 19/12/16 03:56, Jiandi An wrote: > Thanks for you comments. xatp is passed to XEN via the hypervisor call in each loop. > XEN touches xatp and returns it back. For example XEN returns error of underlying mapping call in the err[] array in xatp. (The err[] is not checked after the hypervisor call returns and it's a bug to be addressed in a separate patch) XEN could theoretically corrupt xatp when it's returned. And the loop would go on to the next iteration passing in whatever that's in xatp returned by the previous hypervisor call. Harder to debug in my opinion if xatp get corrupted by XEN somehow when a bug is introduced in XEN. At first I put the memset of xatp at the beginning outside of the loop. But I thought it's better to initialize xatp that's passed in each time a hypervisor call is made so we know exactly we set going into the hypervisor call. If you move struct xen_add_to_physmap_range in the loop, the compiler will initialize and zeroed for you the structure at each loop. I.e for (i = 0; i < count; i++) { struct xen_add_to_physmap_range xapt = .... } Cheers, -- Julien Grall