From mboxrd@z Thu Jan 1 00:00:00 1970 From: Juergen Gross Subject: Re: [PATCH v5 6/9] libxc: create unmapped initrd in domain builder if supported Date: Mon, 30 Nov 2015 12:03:44 +0100 Message-ID: <565C2D10.9060203@suse.com> References: <1447335816-31772-1-git-send-email-jgross@suse.com> <1447335816-31772-7-git-send-email-jgross@suse.com> <5655DDDC.5000005@oracle.com> <5656B626.6080305@suse.com> <20151130102002.GF21588@citrix.com> <565C23A6.4080707@suse.com> <1448879699.15768.14.camel@citrix.com> <565C2946.60109@suse.com> <1448880697.15768.17.camel@citrix.com> <1448880756.15768.18.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1448880756.15768.18.camel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell , Wei Liu Cc: Boris Ostrovsky , stefano.stabellini@eu.citrix.com, ian.jackson@eu.citrix.com, roger.pau@citrix.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 30/11/15 11:52, Ian Campbell wrote: > On Mon, 2015-11-30 at 10:51 +0000, Ian Campbell wrote: >> On Mon, 2015-11-30 at 11:47 +0100, Juergen Gross wrote: >>> On 30/11/15 11:34, Ian Campbell wrote: >>>> On Mon, 2015-11-30 at 11:23 +0100, Juergen Gross wrote: >>>>> On 30/11/15 11:20, Wei Liu wrote: >>>>>> On Thu, Nov 26, 2015 at 08:35:02AM +0100, Juergen Gross wrote: >>>>>>> >>>>>>> /* initrd parameters as specified in start_info page */ >>>>>>> - unsigned long initrd_start; >>>>>>> - unsigned long initrd_len; >>>>>>> + uint64_t initrd_start; >>>>>>> + uint64_t initrd_len; >>>>>>> >>>>>> >>>>>> I think these should be of type xen_vaddr_t. Doesn't make a >>>>>> difference >>>>>> in the end though. >>>>> >>>>> xen_vaddr_t seems not to be appropriate. It can be either a virtual >>>>> address or a pfn. >>>> >>>> Did you mean a virtual address or a physical _address_? Potentially >>>> mixing >>>> addresses and frame numbers in a single variable seems liable to be >>>> confusing, at best. >>> >>> No, it's really a pfn. And this is part of the stable interface between >>> hypervisor and the pv-domU since more than 5 years now. >> >> Including the virtual address bit? >> >> That's a shame. > > ... and that being the case would you mind adding a comment here explaining > the two forms of these variables and the flag which indicates which one is > "in force" at a given moment. The comment in the struct already tells us that initrd_start and initrd_len are in the very same format as in the start_info page. Both fields are meant to be opaque to most of the domain builder parts. The only function dealing with the differences is xc_dom_build_image() which already contains the appropriate flag. I added this on your request. You acked the resulting patch. So why do you want to add another comment now? Juergen