Re: [PATCH 0/7] kvmtool: Cleanup kernel loading
On Wed, Nov 18, 2015 at 10:29:30AM +, Andre Przywara wrote: > On 02/11/15 14:58, Will Deacon wrote: > > On Fri, Oct 30, 2015 at 06:26:53PM +, Andre Przywara wrote: > >> this series cleans up kvmtool's kernel loading functionality a bit. > >> It has been broken out of a previous series I sent [1] and contains > >> just the cleanup and bug fix parts, which should be less controversial > >> and thus easier to merge ;-) > >> I will resend the pipe loading part later on as a separate series. > >> > >> The first patch properly abstracts kernel loading to move > >> responsibility into each architecture's code. It removes quite some > >> ugly code from the generic kvm.c file. > >> The later patches address the naive usage of read(2) to, well, read > >> data from files. Doing this without coping with the subtleties of > >> the UNIX read semantics (returning with less or none data read is not > >> an error) can provoke hard to debug failures. > >> So these patches make use of the existing and one new wrapper function > >> to make sure we read everything we actually wanted to. > >> The last patch moves the ARM kernel loading code into the proper > >> location to be in line with the other architectures. > >> > >> Please have a look and give some comments! > > > > Looks good to me, but I'd like to see some comments from some mips/ppc/x86 > > people on the changes you're making over there. > > Sounds reasonable, but no answers yet. > > Can you take at least patch 1 and 2 meanwhile, preferably 6 and 7 (the > ARM parts) also if you are OK with it? > I have other patches that depend on 1/7 and 2/7, so having them upstream > would help me and reduce further dependency churn. > I am happy to resend the remaining patches for further discussion later. We let them sit on the list for a while with no comments, so I just pushed out your series. If a bug report shows up, then we can always revert the offending patch if there's no quick fix. Will -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/7] kvmtool: Cleanup kernel loading
Hi Will, On 02/11/15 14:58, Will Deacon wrote: > On Fri, Oct 30, 2015 at 06:26:53PM +, Andre Przywara wrote: >> Hi, > > Hello Andre, > >> this series cleans up kvmtool's kernel loading functionality a bit. >> It has been broken out of a previous series I sent [1] and contains >> just the cleanup and bug fix parts, which should be less controversial >> and thus easier to merge ;-) >> I will resend the pipe loading part later on as a separate series. >> >> The first patch properly abstracts kernel loading to move >> responsibility into each architecture's code. It removes quite some >> ugly code from the generic kvm.c file. >> The later patches address the naive usage of read(2) to, well, read >> data from files. Doing this without coping with the subtleties of >> the UNIX read semantics (returning with less or none data read is not >> an error) can provoke hard to debug failures. >> So these patches make use of the existing and one new wrapper function >> to make sure we read everything we actually wanted to. >> The last patch moves the ARM kernel loading code into the proper >> location to be in line with the other architectures. >> >> Please have a look and give some comments! > > Looks good to me, but I'd like to see some comments from some mips/ppc/x86 > people on the changes you're making over there. Sounds reasonable, but no answers yet. Can you take at least patch 1 and 2 meanwhile, preferably 6 and 7 (the ARM parts) also if you are OK with it? I have other patches that depend on 1/7 and 2/7, so having them upstream would help me and reduce further dependency churn. I am happy to resend the remaining patches for further discussion later. Cheers, Andre. -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/7] kvmtool: Cleanup kernel loading
On 2 November 2015 at 14:58, Will Deaconwrote: > On Fri, Oct 30, 2015 at 06:26:53PM +, Andre Przywara wrote: >> Hi, > > Hello Andre, > >> this series cleans up kvmtool's kernel loading functionality a bit. >> It has been broken out of a previous series I sent [1] and contains >> just the cleanup and bug fix parts, which should be less controversial >> and thus easier to merge ;-) >> I will resend the pipe loading part later on as a separate series. >> >> The first patch properly abstracts kernel loading to move >> responsibility into each architecture's code. It removes quite some >> ugly code from the generic kvm.c file. >> The later patches address the naive usage of read(2) to, well, read >> data from files. Doing this without coping with the subtleties of >> the UNIX read semantics (returning with less or none data read is not >> an error) can provoke hard to debug failures. >> So these patches make use of the existing and one new wrapper function >> to make sure we read everything we actually wanted to. >> The last patch moves the ARM kernel loading code into the proper >> location to be in line with the other architectures. >> >> Please have a look and give some comments! > > Looks good to me, but I'd like to see some comments from some mips/ppc/x86 > people on the changes you're making over there. Looks mostly good to me, as one of the kvmtool down streams. Over at https://github.com/clearlinux/kvmtool we have some patches to tweak the x86 boot flow, which will need rebasing/retweaking) specifically this commit here - https://github.com/clearlinux/kvmtool/commit/a8dee709f85735d16739d0eda0cc00d3c1b17477 -- Regards, Dimitri. 53 sleeps till Christmas, or less https://clearlinux.org Open Source Technology Center Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ. -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/7] kvmtool: Cleanup kernel loading
On Fri, Oct 30, 2015 at 06:26:53PM +, Andre Przywara wrote: > Hi, Hello Andre, > this series cleans up kvmtool's kernel loading functionality a bit. > It has been broken out of a previous series I sent [1] and contains > just the cleanup and bug fix parts, which should be less controversial > and thus easier to merge ;-) > I will resend the pipe loading part later on as a separate series. > > The first patch properly abstracts kernel loading to move > responsibility into each architecture's code. It removes quite some > ugly code from the generic kvm.c file. > The later patches address the naive usage of read(2) to, well, read > data from files. Doing this without coping with the subtleties of > the UNIX read semantics (returning with less or none data read is not > an error) can provoke hard to debug failures. > So these patches make use of the existing and one new wrapper function > to make sure we read everything we actually wanted to. > The last patch moves the ARM kernel loading code into the proper > location to be in line with the other architectures. > > Please have a look and give some comments! Looks good to me, but I'd like to see some comments from some mips/ppc/x86 people on the changes you're making over there. Will -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/7] kvmtool: Cleanup kernel loading
Hi Dimitri, On 02/11/15 15:17, Dimitri John Ledkov wrote: > On 2 November 2015 at 14:58, Will Deaconwrote: >> On Fri, Oct 30, 2015 at 06:26:53PM +, Andre Przywara wrote: >>> Hi, >> >> Hello Andre, >> >>> this series cleans up kvmtool's kernel loading functionality a bit. >>> It has been broken out of a previous series I sent [1] and contains >>> just the cleanup and bug fix parts, which should be less controversial >>> and thus easier to merge ;-) >>> I will resend the pipe loading part later on as a separate series. >>> >>> The first patch properly abstracts kernel loading to move >>> responsibility into each architecture's code. It removes quite some >>> ugly code from the generic kvm.c file. >>> The later patches address the naive usage of read(2) to, well, read >>> data from files. Doing this without coping with the subtleties of >>> the UNIX read semantics (returning with less or none data read is not >>> an error) can provoke hard to debug failures. >>> So these patches make use of the existing and one new wrapper function >>> to make sure we read everything we actually wanted to. >>> The last patch moves the ARM kernel loading code into the proper >>> location to be in line with the other architectures. >>> >>> Please have a look and give some comments! >> >> Looks good to me, but I'd like to see some comments from some mips/ppc/x86 >> people on the changes you're making over there. > > Looks mostly good to me, as one of the kvmtool down streams. Over at > https://github.com/clearlinux/kvmtool we have some patches to tweak > the x86 boot flow, which will need rebasing/retweaking) specifically > this commit here - > https://github.com/clearlinux/kvmtool/commit/a8dee709f85735d16739d0eda0cc00d3c1b17477 Awesome - I was actually thinking about coding something like this! In the last week I move the MIPS ELF loading out of mips/ into /elf.c to be able to load kvm-unit-tests' tests (which are Multiboot/ELF). As multiboot requires entering in protected mode, I was thinking about changing kvmtool to support entering a guest directly in protected mode - seems like this is mostly what you've done here already. Looks like we should both post our patches to merge them somehow ;-) Cheers, Andre. -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/7] kvmtool: Cleanup kernel loading
Hi, this series cleans up kvmtool's kernel loading functionality a bit. It has been broken out of a previous series I sent [1] and contains just the cleanup and bug fix parts, which should be less controversial and thus easier to merge ;-) I will resend the pipe loading part later on as a separate series. The first patch properly abstracts kernel loading to move responsibility into each architecture's code. It removes quite some ugly code from the generic kvm.c file. The later patches address the naive usage of read(2) to, well, read data from files. Doing this without coping with the subtleties of the UNIX read semantics (returning with less or none data read is not an error) can provoke hard to debug failures. So these patches make use of the existing and one new wrapper function to make sure we read everything we actually wanted to. The last patch moves the ARM kernel loading code into the proper location to be in line with the other architectures. Please have a look and give some comments! Find the branch on my kvmtool git tree on: git://linux-arm.org/kvmtool.git (kern_load-v2 branch) http://www.linux-arm.org/git?p=kvmtool.git;a=shortlog;h=refs/heads/kern_load-v2 Cheers, Andre. [1] http://marc.info/?l=kvm=143825354808135=2 Andre Przywara (7): Refactor kernel image loading provide generic read_file() implementation powerpc: use read_file() in kernel and initrd loading MIPS: use read wrappers in kernel loading x86: use read wrappers in kernel loading arm/arm64: use read_file() in kernel and initrd loading arm: move kernel loading into arm/kvm.c arm/fdt.c| 99 +--- arm/kvm.c| 88 ++ include/kvm/kvm.h| 5 +-- include/kvm/read-write.h | 2 + kvm.c| 42 ++-- mips/kvm.c | 57 ++-- powerpc/kvm.c| 39 ++- util/read-write.c| 21 ++ x86/kvm.c| 62 +++--- 9 files changed, 207 insertions(+), 208 deletions(-) -- 2.5.1 -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html