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 13:59:39 +0100 Message-ID: <565C483B.1080908@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> <565C2D10.9060203@suse.com> <1448882606.15768.21.camel@citrix.com> <565C3EFD.40208@suse.com> <1448886931.15768.31.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1448886931.15768.31.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 , roger.pau@citrix.com, ian.jackson@eu.citrix.com, xen-devel@lists.xen.org, stefano.stabellini@eu.citrix.com List-Id: xen-devel@lists.xenproject.org On 30/11/15 13:35, Ian Campbell wrote: > On Mon, 2015-11-30 at 13:20 +0100, Juergen Gross wrote: >> On 30/11/15 12:23, Ian Campbell wrote: >>> On Mon, 2015-11-30 at 12:03 +0100, Juergen Gross wrote: >>>> 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? >>> >>> I hadn't realised at the time that the semantics of these fields was >>> so, >>> uh, interesting. >> >> :-) >> >> I guess due to the lack of a comment? ;-) > > ;-) > >> Okay, I'll add one when submitting the patch after (hopefully) Boris >> confirmed it is fixing his problem. > > Thanks! > > FYI attempting to upgrade osstest to use Debian Jessie in the guest seems > to have exposed another issue here. > > http://logs.test-lab.xenproject.org/osstest/logs/65172/test-amd64-amd64-amd64-pvgrub/info.html > > Booting 'Debian GNU/Linux, kernel 3.16.0-4-amd64' > > root (hd0,0) > Filesystem type is ext2fs, partition type 0x83 > kernel /boot/vmlinuz-3.16.0-4-amd64 root=UUID=12447529-e85a-4b41-86b6-3e83ccfc > 1377 ro > initrd /boot/initrd.img-3.16.0-4-amd64 > > ============= Init TPM Front ================ > Tpmfront:Error Unable to read device/vtpm/0/backend-id during tpmfront initialization! error = ENOENT > Tpmfront:Info Shutting down tpmfront > pin_table(x) returned 1357193 > close(3) > > Error 9: Unknown boot failure > > Press any key to continue... > > xen.git 6853c9bf9ff0 is OK, whereas 713b7e4ef2aa is not. Adding your two > outstanding patches: > libxc: correct domain builder for 64 bit guest with 32 bit tools > libxc: use correct return type for do_memory_op() > > Doesn't appear to have helped. Anyway, I was in the process of > investigating/bisecting etc but since I was mailing you any way I thought > I'd mention it. I'll start a fresh thread once I have some more to go on. When something was wrong with pvgrub in my tests of the patches it died right away and didn't show random errors later. I don't think the problem you are seeing is related to my recent changes. OTOH I have been wrong before. :-( Juergen