Re: [Qemu-devel] [Qemu-ppc] [PATCH] PPC64: Add support for ldbrx and stdbrx instructions
Am Wed, 8 Feb 2012 21:48:40 +1100 schrieb David Gibson : > On Wed, Feb 08, 2012 at 10:54:21AM +0400, malc wrote: > > On Wed, 8 Feb 2012, David Gibson wrote: > > > > > From: Thomas Huth > > > > > > These instructions for loading and storing byte-swapped 64-bit values have > > > been introduced in PowerISA 2.06. > > > > > > Signed-off-by: Thomas Huth > > > --- > > > target-ppc/translate.c | 30 ++ > > > 1 files changed, 30 insertions(+), 0 deletions(-) > > > > I seem to recall that POWER5 machine i had access to didn't have > > ld/stdbrx while CBE did have it (or was it the other way around?) > > so question is - is PPC_64B sufficient? > > Ah, I think it's not. I think I spotted that before, but then forgot > about it. Thanks for the reminder. Maybe it's a better idea to use PPC_64BX here? ... but that flag seems to be missing in POWERPC_INSNS_POWER7... David, could PPC_64BX also be included in that flag list? Thomas
Re: [Qemu-devel] [Qemu-ppc] [PATCH] PPC64: Add support for ldbrx and stdbrx instructions
Am Thu, 9 Feb 2012 11:26:09 +1100 schrieb David Gibson : > On Wed, Feb 08, 2012 at 02:27:35PM +0100, Thomas Huth wrote: > > Am Wed, 8 Feb 2012 21:48:40 +1100 > > schrieb David Gibson : > > > > > On Wed, Feb 08, 2012 at 10:54:21AM +0400, malc wrote: > > > > On Wed, 8 Feb 2012, David Gibson wrote: > > > > > > > > > From: Thomas Huth > > > > > > > > > > These instructions for loading and storing byte-swapped 64-bit values > > > > > have > > > > > been introduced in PowerISA 2.06. > > > > > > > > > > Signed-off-by: Thomas Huth > > > > > --- > > > > > target-ppc/translate.c | 30 ++ > > > > > 1 files changed, 30 insertions(+), 0 deletions(-) > > > > > > > > I seem to recall that POWER5 machine i had access to didn't have > > > > ld/stdbrx while CBE did have it (or was it the other way around?) > > > > so question is - is PPC_64B sufficient? > > > > > > Ah, I think it's not. I think I spotted that before, but then forgot > > > about it. Thanks for the reminder. > > > > Maybe it's a better idea to use PPC_64BX here? ... but that flag seems > > to be missing in POWERPC_INSNS_POWER7... David, could PPC_64BX also be > > included in that flag list? > > Um.. what exactly do you mean by 64BX? > There is a definition in target-ppc/cpu.h: /* New 64 bits extensions (PowerPC 2.0x) */ PPC_64BX = 0x0040ULL, According to the comment, I thought the PPC_64BX might be designed for new 64-bit instructions? Thomas
Re: [Qemu-devel] [Qemu-ppc] RFC: NVRAM for pseries machine
Am Wed, 26 Sep 2012 10:56:07 +0200 schrieb Alexander Graf : > > > On 26.09.2012, at 03:18, David Gibson wrote: > > > On Wed, Sep 26, 2012 at 03:03:10AM +0200, Alexander Graf wrote: [snip] > >> spapr-nvram: > >> > >> if (!drive || checksum_is_bad(drive)) > >>autogenerate_nvram_contents(); > > > > Actually, I'm planning for the initialization of the content to be > > done from the guest firmware. > > Does the guest have all information necessary to construct a workable nvram > image? If so, then yes, that's even better. At least SLOF already contains the code to initialize the PAPR-style NVRAM partitions from scratch. So as soon as SLOF can work with this new NVRAM devices, it should be able to initialize the required layout. Thomas
Re: [Qemu-devel] [RFC patch 5/6] s390: implement pci instruction
Hi Frank,
On Fri, 19 Sep 2014 13:54:34 +0200
frank.blasc...@de.ibm.com wrote:
> From: Frank Blaschka
>
> This patch implements the s390 pci instructions in qemu. This allows
> to attach qemu pci devices including vfio. This does not mean the
> devices are functional but at least detection and config/memory space
> access is working.
>
> Signed-off-by: Frank Blaschka
> ---
> target-s390x/Makefile.objs |2
> target-s390x/kvm.c | 52 +++
> target-s390x/pci_ic.c | 621
> +
> target-s390x/pci_ic.h | 425 ++
> 4 files changed, 1099 insertions(+), 1 deletion(-)
>
> --- a/target-s390x/Makefile.objs
> +++ b/target-s390x/Makefile.objs
> @@ -2,4 +2,4 @@ obj-y += translate.o helper.o cpu.o inte
> obj-y += int_helper.o fpu_helper.o cc_helper.o mem_helper.o misc_helper.o
> obj-y += gdbstub.o
> obj-$(CONFIG_SOFTMMU) += ioinst.o arch_dump.o
> -obj-$(CONFIG_KVM) += kvm.o
> +obj-$(CONFIG_KVM) += kvm.o pci_ic.o
> --- a/target-s390x/kvm.c
> +++ b/target-s390x/kvm.c
> @@ -40,6 +40,7 @@
> #include "exec/gdbstub.h"
> #include "trace.h"
> #include "qapi-event.h"
> +#include "pci_ic.h"
>
> /* #define DEBUG_KVM */
>
> @@ -56,6 +57,7 @@
> #define IPA0_B2 0xb200
> #define IPA0_B9 0xb900
> #define IPA0_EB 0xeb00
> +#define IPA0_E3 0xe300
>
> #define PRIV_B2_SCLP_CALL 0x20
> #define PRIV_B2_CSCH0x30
> @@ -76,8 +78,17 @@
> #define PRIV_B2_XSCH0x76
>
> #define PRIV_EB_SQBS0x8a
> +#define PRIV_EB_PCISTB 0xd0
> +#define PRIV_EB_SIC 0xd1
>
> #define PRIV_B9_EQBS0x9c
> +#define PRIV_B9_CLP 0xa0
> +#define PRIV_B9_PCISTG 0xd0
> +#define PRIV_B9_PCILG 0xd2
> +#define PRIV_B9_RPCIT 0xd3
> +
> +#define PRIV_E3_MPCIFC 0xd0
> +#define PRIV_E3_STPCIFC 0xd4
>
> #define DIAG_IPL0x308
> #define DIAG_KVM_HYPERCALL 0x500
> @@ -813,6 +824,18 @@ static int handle_b9(S390CPU *cpu, struc
> int r = 0;
>
> switch (ipa1) {
> +case PRIV_B9_CLP:
> +r = kvm_clp_service_call(cpu, run);
> +break;
> +case PRIV_B9_PCISTG:
> +r = kvm_pcistg_service_call(cpu, run);
> +break;
> +case PRIV_B9_PCILG:
> +r = kvm_pcilg_service_call(cpu, run);
> +break;
> +case PRIV_B9_RPCIT:
> +r = kvm_rpcit_service_call(cpu, run);
> +break;
> case PRIV_B9_EQBS:
> /* just inject exception */
> r = -1;
> @@ -831,6 +854,12 @@ static int handle_eb(S390CPU *cpu, struc
> int r = 0;
>
> switch (ipa1) {
> +case PRIV_EB_PCISTB:
> +r = kvm_pcistb_service_call(cpu, run);
> +break;
> +case PRIV_EB_SIC:
> +r = kvm_sic_service_call(cpu, run);
> +break;
> case PRIV_EB_SQBS:
> /* just inject exception */
> r = -1;
I'm not sure, but I think the handler for the eb instructions is wrong:
The second byte of the opcode is encoded in the lowest byte of the ipb
field, not the lowest byte of the ipa field (just like with the e3
handler). Did you verify that your handlers get called correctly?
> @@ -844,6 +873,26 @@ static int handle_eb(S390CPU *cpu, struc
> return r;
> }
>
> +static int handle_e3(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1)
> +{
> +int r = 0;
> +
> +switch (ipa1) {
> +case PRIV_E3_MPCIFC:
> +r = kvm_mpcifc_service_call(cpu, run);
> +break;
> +case PRIV_E3_STPCIFC:
> +r = kvm_stpcifc_service_call(cpu, run);
> +break;
> +default:
> +r = -1;
> +DPRINTF("KVM: unhandled PRIV: 0xe3%x\n", ipa1);
> +break;
> +}
> +
> +return r;
> +}
Could you please replace "ipa1" with "ipb1" to avoid confusion here?
> static int handle_hypercall(S390CPU *cpu, struct kvm_run *run)
> {
> CPUS390XState *env = &cpu->env;
> @@ -1038,6 +1087,9 @@ static int handle_instruction(S390CPU *c
> case IPA0_EB:
> r = handle_eb(cpu, run, ipa1);
> break;
> +case IPA0_E3:
> +r = handle_e3(cpu, run, run->s390_sieic.ipb & 0xff);
> +break;
> case IPA0_DIAG:
> r = handle_diag(cpu, run, run->s390_sieic.ipb);
> break;
> --- /dev/null
> +++ b/target-s390x/pci_ic.c
> @@ -0,0 +1,621 @@
[...]
> +
> +int kvm_pcilg_service_call(S390CPU *cpu, struct kvm_run *run)
> +{
> +CPUS390XState *env = &cpu->env;
> +S390PCIBusDevice *pbdev;
> +uint8_t r1 = (run->s390_sieic.ipb & 0x00f0) >> 20;
> +uint8_t r2 = (run->s390_sieic.ipb & 0x000f) >> 16;
> +PciLgStg *rp;
> +uint64_t offset;
> +uint64_t data;
> +
> +cpu_synchronize_state(CPU(cpu));
> +rp = (PciLgStg *)&env->re
Re: [Qemu-devel] [Qemu-ppc] [PATCH 2/2] [v3] target-ppc: Enhance CPU nodes of device tree to be PAPR compliant.
Am Mon, 12 Aug 2013 16:03:24 +1000 schrieb Benjamin Herrenschmidt : > On Mon, 2013-08-12 at 10:07 +0530, Prerna Saxena wrote: > > .../... > > > I dont know what context lead to this observation. > > However, PAPR mentions the following nomenclature guideline: > > > > "The value of this property shall be of the form: “PowerPC,”, > > where is the name of the processor chip which may be displayed to > > the user. shall not contain underscores." > > This actually comes from the original Open Firmware binding for PowerPC > processors, which PAPR inherits largely from. Thus this naming scheme > should apply to all PowerPC processors when a device-tree is involved. Well, I think it should be used when an Open Firmware environment is used. When you boot via ePAPR device tree, the name should be "cpu" instead, according to the ePAPR specification. Thomas
Re: [Qemu-devel] My OS hangup in KVM for some reasons, how can I debug?
On Sun, 11 May 2014 21:09:44 +0800 Jun Koi wrote: > On Fri, May 9, 2014 at 11:24 AM, Jun Koi wrote: > > > > > > > > > On Thu, May 8, 2014 at 4:28 PM, Jun Koi wrote: > > > >> Hi, > >> > >> I have an weird OS that I am trying to boot in KVM. however, it just hang > >> in the middle, without a good reason. > >> > >> The same OS boots fine in physical machine, and this OS comes with no > >> source code. > >> > >> There must be a bug somewhere in KVM, so I am wondering how people debug > >> deal with such a case to find and fix bugs in KVM? > >> > >> Same thing like this happened before with some new OSes (such as Windows > >> 8), and somebody always came up with a quick fix, so I am wondering what > >> the magic is here. > >> > >> > > > Any help, please? You could try to attach a GDB to see where it is hanging (start qemu with the -s option, see http://wiki.qemu.org/Documentation/Debugging). Thomas
Re: [Qemu-devel] [PATCH] kvm: Fix enable_cap helpers on older gcc
On Sun, 11 May 2014 18:11:04 +0200
Alexander Graf wrote:
> Commit 40f1ee27aa1 introduced handy helpers for enable_cap calls on
> vcpu and vm level. Unfortunately some older gcc versions (4.7.1, 4.6)
> seem to choke on signedness detection in inline created variables:
>
> target-ppc/kvm.c: In function 'kvmppc_booke_watchdog_enable':
> target-ppc/kvm.c:1302:21: error: comparison of unsigned expression < 0 is
> always false [-Werror=type-limits]
> target-ppc/kvm.c: In function 'kvmppc_set_papr':
> target-ppc/kvm.c:1504:21: error: comparison of unsigned expression < 0 is
> always false [-Werror=type-limits]
>
> We can easily work around this by not doing a compare with 0, but instead
> shift all values up by one. The resulting binary should look the same, as
> all those ARRAY_SIZE values are compile time statically optimized.
>
> This patch fixes compile errors on some of my systems.
>
> Signed-off-by: Alexander Graf
> ---
> include/sysemu/kvm.h | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index 192fe89..989c261 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -302,9 +302,9 @@ int kvm_check_extension(KVMState *s, unsigned int
> extension);
> }; \
> uint64_t args_tmp[] = { __VA_ARGS__ }; \
> int i; \
> -for (i = 0; i < ARRAY_SIZE(args_tmp) && \
> - i < ARRAY_SIZE(cap.args); i++) {\
> -cap.args[i] = args_tmp[i]; \
> +for (i = 1; i < (MIN(ARRAY_SIZE(args_tmp), \
> + ARRAY_SIZE(cap.args)) + 1); i++) { \
> +cap.args[i - 1] = args_tmp[i - 1]; \
> }\
> kvm_vm_ioctl(s, KVM_ENABLE_CAP, &cap); \
> })
> @@ -317,9 +317,9 @@ int kvm_check_extension(KVMState *s, unsigned int
> extension);
> }; \
> uint64_t args_tmp[] = { __VA_ARGS__ }; \
> int i; \
> -for (i = 0; i < ARRAY_SIZE(args_tmp) && \
> - i < ARRAY_SIZE(cap.args); i++) {\
> -cap.args[i] = args_tmp[i]; \
> +for (i = 1; i < (MIN(ARRAY_SIZE(args_tmp), \
> + ARRAY_SIZE(cap.args)) + 1); i++) { \
> +cap.args[i - 1] = args_tmp[i - 1]; \
> }\
> kvm_vcpu_ioctl(cpu, KVM_ENABLE_CAP, &cap); \
> })
That looks ... somewhat ugly. I think you either should add a proper
comment here (or somebody will fix it back in the future when reading
the code), ... or would it also work to type-cast the result of
ARRAY_SIZE to "int"? Something like this:
for (i = 0; i < (int)ARRAY_SIZE(args_tmp) && \
i < (int)ARRAY_SIZE(cap.args); i++)
Thomas
Re: [Qemu-devel] [PATCH v2] kvm: Fix enable_cap helpers on older gcc
On Mon, 12 May 2014 11:51:37 +0200
Alexander Graf wrote:
> Commit 40f1ee27aa1 introduced handy helpers for enable_cap calls on
> vcpu and vm level. Unfortunately some older gcc versions (4.7.1, 4.6)
> seem to choke on signedness detection in inline created variables:
>
> target-ppc/kvm.c: In function 'kvmppc_booke_watchdog_enable':
> target-ppc/kvm.c:1302:21: error: comparison of unsigned expression < 0 is
> always false [-Werror=type-limits]
> target-ppc/kvm.c: In function 'kvmppc_set_papr':
> target-ppc/kvm.c:1504:21: error: comparison of unsigned expression < 0 is
> always false [-Werror=type-limits]
>
> However - thanks to Thomas Huth for the suggestion - we can just cast the
> offending potentially 0 value to a signed type, making the comparison signed.
>
> Signed-off-by: Alexander Graf
>
> ---
>
> v1 -> v2:
>
> - only do cast, leaving readability intact
>
> ---
> include/sysemu/kvm.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index 192fe89..76c39cc 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -302,7 +302,7 @@ int kvm_check_extension(KVMState *s, unsigned int
> extension);
> }; \
> uint64_t args_tmp[] = { __VA_ARGS__ }; \
> int i; \
> -for (i = 0; i < ARRAY_SIZE(args_tmp) && \
> +for (i = 0; i < (int)ARRAY_SIZE(args_tmp) && \
> i < ARRAY_SIZE(cap.args); i++) {\
> cap.args[i] = args_tmp[i]; \
> }\
> @@ -317,7 +317,7 @@ int kvm_check_extension(KVMState *s, unsigned int
> extension);
> }; \
> uint64_t args_tmp[] = { __VA_ARGS__ }; \
> int i; \
> -for (i = 0; i < ARRAY_SIZE(args_tmp) && \
> +for (i = 0; i < (int)ARRAY_SIZE(args_tmp) && \
> i < ARRAY_SIZE(cap.args); i++) {\
> cap.args[i] = args_tmp[i]; \
> }\
Looks much better this way :-)
Reviewed-by: Thomas Huth
Re: [Qemu-devel] [PULL 7/8] s390x/migration: migrate CPU state
On Thu, 9 Oct 2014 17:28:57 +0100
Peter Maydell wrote:
> On 9 October 2014 14:36, Cornelia Huck wrote:
> > From: Thomas Huth
> >
> > This patch provides the cpu save information for dumps and later life
> > migration and enables migration of the CPU state. The code is based on
> > earlier work from Christian Borntraeger and Jason Herne.
> >
> > Signed-off-by: Thomas Huth
> > Signed-off-by: David Hildenbrand
> > [provide cpu_post_load()]
> > Signed-off-by: Jens Freimann
> > CC: Andreas Faerber
> > CC: Christian Borntraeger
> > CC: Jason J. Herne
> > Tested-by: Christian Borntraeger
> > Signed-off-by: Cornelia Huck
> > ---
> > target-s390x/cpu.c | 59
> > ++--
> > 1 file changed, 57 insertions(+), 2 deletions(-)
> >
> > diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> > index ec7df90..c9c237f 100644
> > --- a/target-s390x/cpu.c
> > +++ b/target-s390x/cpu.c
>
> I think the migration code should live in machine.c like
> it does for our other targets. (Among other useful things,
> this means you can have the makefile say
>obj-$(CONFIG_SOFTMMU) += machine.o
> so it doesn't try to build it for the linux-user target :-))
We originally had that file for s390x, too, but it had been removed by
Andreas with this commit: c7396bbb2597577b1463fc997a73e67b8a067880
I guess we could re-introduce it again now that we have some meaningful
contents for the file. Andreas, is that ok for you?
> > static const VMStateDescription vmstate_s390_cpu = {
> > .name = "cpu",
> > -.unmigratable = 1,
> > +.post_load = cpu_post_load,
> > +.version_id = 1,
> > +.minimum_version_id = 1,
> > +.minimum_version_id_old = 1,
>
> You don't need minimum_version_id_old any more.
Ok, I'll remove it.
> > +.fields = (VMStateField[]) {
> > +VMSTATE_UINT64(env.fregs[0].ll, S390CPU),
> > +VMSTATE_UINT64(env.fregs[1].ll, S390CPU),
> > +VMSTATE_UINT64(env.fregs[2].ll, S390CPU),
> > +VMSTATE_UINT64(env.fregs[3].ll, S390CPU),
> > +VMSTATE_UINT64(env.fregs[4].ll, S390CPU),
> > +VMSTATE_UINT64(env.fregs[5].ll, S390CPU),
> > +VMSTATE_UINT64(env.fregs[6].ll, S390CPU),
> > +VMSTATE_UINT64(env.fregs[7].ll, S390CPU),
> > +VMSTATE_UINT64(env.fregs[8].ll, S390CPU),
> > +VMSTATE_UINT64(env.fregs[9].ll, S390CPU),
> > +VMSTATE_UINT64(env.fregs[10].ll, S390CPU),
> > +VMSTATE_UINT64(env.fregs[11].ll, S390CPU),
> > +VMSTATE_UINT64(env.fregs[12].ll, S390CPU),
> > +VMSTATE_UINT64(env.fregs[13].ll, S390CPU),
> > +VMSTATE_UINT64(env.fregs[14].ll, S390CPU),
> > +VMSTATE_UINT64(env.fregs[15].ll, S390CPU),
> > +VMSTATE_UINT64_ARRAY(env.regs, S390CPU, 16),
> > +VMSTATE_UINT64(env.psw.mask, S390CPU),
> > +VMSTATE_UINT64(env.psw.addr, S390CPU),
> > +VMSTATE_UINT64(env.psa, S390CPU),
> > +VMSTATE_UINT32(env.fpc, S390CPU),
> > +VMSTATE_UINT32(env.todpr, S390CPU),
> > +VMSTATE_UINT64(env.pfault_token, S390CPU),
> > +VMSTATE_UINT64(env.pfault_compare, S390CPU),
> > +VMSTATE_UINT64(env.pfault_select, S390CPU),
> > +VMSTATE_UINT64(env.cputm, S390CPU),
> > +VMSTATE_UINT64(env.ckc, S390CPU),
> > +VMSTATE_UINT64(env.gbea, S390CPU),
> > +VMSTATE_UINT64(env.pp, S390CPU),
> > +VMSTATE_UINT32_ARRAY(env.aregs, S390CPU, 16),
> > +VMSTATE_UINT64_ARRAY(env.cregs, S390CPU, 16),
> > +VMSTATE_UINT8(env.cpu_state, S390CPU),
> > +VMSTATE_END_OF_LIST()
> > + },
> > +.subsections = (VMStateSubsection[]) {
> > +{
> > +/* empty */
> > +}
>
> Why the empty subsections list?
Likely an old copy-n-paste error ... I'll remove that, too.
Thomas
Re: [Qemu-devel] [PATCH] get_maintainer.pl: Default to --no-git-fallback
On Mon, 20 Oct 2014 17:15:48 +0300 "Michael S. Tsirkin" wrote: > On Mon, Oct 20, 2014 at 03:04:44PM +0100, Peter Maydell wrote: > > On 20 October 2014 10:19, Markus Armbruster wrote: > > > Contributors rely on this script to find maintainers to copy. The > > > script falls back to git when no exact MAINTAINERS pattern matches. > > > When that happens, recent contributors get copied, which tends not be > > > particularly useful. Some contributors find it even annoying. > > > > > > Flip the default to "don't fall back to git". Use --git-fallback to > > > ask it to fall back to git. > > > > > > Signed-off-by: Markus Armbruster > > > > Good idea. > > > > Reviewed-by: Peter Maydell > > > > -- PMM > > What do you want to happen in this case? > Won't this cause even more patches to fall to the floor? > > The benefit seems marginal, the risk high. Well, IMHO, at least the current behaviour if git-fallback seems to be a little bit too easygoing: I already got mails just because I once reviewed a patch in the past and thus got listed with the "Reviewed-by:" tag. I would not expect that behaviour when I run a "get_maintainers" script (it's not called "get_reviewers", is it?). Maybe you could at least remove the "Reviewed-by:" and "Acked-by:" from the --git-fallback option so that it only checks for the SOBs? Thomas
Re: [Qemu-devel] [PATCH] get_maintainer.pl: Default to --no-git-fallback
On Wed, 22 Oct 2014 10:01:43 +0300 "Michael S. Tsirkin" wrote: > On Wed, Oct 22, 2014 at 08:39:59AM +0200, Markus Armbruster wrote: > > "Michael S. Tsirkin" writes: > > > > > On Tue, Oct 21, 2014 at 03:29:14PM +0200, Markus Armbruster wrote: > > >> "Michael S. Tsirkin" writes: > > >> > > >> > On Tue, Oct 21, 2014 at 02:22:41PM +0200, Markus Armbruster wrote: > > [...] > > >> >> My patch to get_maintainers.pl triggered a whole thread, while the > > >> >> message I sent on MAINTAINERS coverage got just one reply so far, and > > >> >> even that one's really just about get_maintainers.pl. Disappointing. > > >> >> Looks like we're still looking for an easy technical fix. I doubt > > >> >> there > > >> >> is one. > > >> > > > >> > At least for myself, that's because I'm Cc'd directly on the patch > > >> > but not on the MAINTAINERS coverage mail. > > >> > And that's ... because get_maintainers picks my mail from git? > > >> > > > >> > See how it's useful now? > > >> > > >> Except that's not what happened. > > >> > > >> $ scripts/get_maintainer.pl --git-fallback -f > > >> scripts/get_maintainer.pl > > >> > > >> No output. I picked you from git-log manually. > > > > > > Weird. > > > It works for me: > > > ./scripts/get_maintainer.pl -f scripts/get_maintainer.pl > > > "Michael S. Tsirkin" (commit_signer:1/1=100%) > > > > > > Maybe --git-fallback is broken? > > > > I tried on master (i.e. without my patch, clean tree, with and without > > --git-fallback. Just tried it again, same result. > > Well ... I don't know why. > It's clearly a bug. > Different perl version? Different git version? > > Can you try tracing it? > Does it exec git? I've got the same behaviour as Markus (i.e. no output), and I think this is due to get_maintainer.pl using $email_git_since = "1-year-ago" ... since there wasn't a commit to that file in the last year, git-log simply does not output any entry at all. Do you have a non-upstream commit in your git tree that changes that file? That would explain why you get some output here. Thomas
Re: [Qemu-devel] [PATCH 4/4] get_maintainer.pl: point at --git-fallback instead of enabling it automatically
On Wed, 22 Oct 2014 11:56:30 +0200 Paolo Bonzini wrote: > > > On 10/22/2014 11:33 AM, Michael S. Tsirkin wrote: > > On Wed, Oct 22, 2014 at 11:08:22AM +0200, Paolo Bonzini wrote: > >> The list emitted by --git-fallback often leads inexperienced contributors > >> to add pointless CCs. While not discouraging usage of --git-fallback, > >> we want to warn the contributors about using their common sense. > >> > >> So, default to *not* enabling --git-fallback, but print a message if > >> none of the files has a match against MAINTAINERS. Of course the > >> message is hidden by --no-git-fallback. > >> > >> Examples: > >> > >> 1) No maintainer for all specified files, print message: > >> > >> $ scripts/get_maintainer.pl -f util/cutils.c > >> No maintainers found. > >> You may want to try --git-fallback to find recent contributors. > >> Do not blindly cc: them on patches! Use common sense. > >> > > > > Does it make sense for util/cutils.c? > > I doubt it, so we are just giving useless advice? > > > > --git-blame might be a better fallback here? > > How about an entry in MAINTAINERS to trigger git-blame? > > We cannot know which is better. The right thing to do would be to use > git-blame *manually*, so as to find who touched the function you are > touching now. Maybe you could at least also add "--git-blame" to the message that is printed out here - so in case --git-fallback does not print anything at all due to the 1-year limitation, the user has at least another thing to try? Thomas
Re: [Qemu-devel] [PATCH 0/4] small improvements to get_maintainer.pl
On Wed, 22 Oct 2014 11:08:18 +0200
Paolo Bonzini wrote:
> See individual patches. My Perl-fu is limited, but the individual
> changes are easy enough.
>
> Paolo Bonzini (4):
> get_maintainer.pl: exit with status 1 if no maintainer found
> get_maintainer.pl: treat all M entries the same
> get_maintainer.pl: move git loop under "if ($email) {"
> get_maintainer.pl: point at --git-fallback instead of enabling it
> automatically
>
> scripts/get_maintainer.pl | 52
> +++
> 1 file changed, 21 insertions(+), 31 deletions(-)
I think these are good patches ... but while you're at it, I wonder
whether you should/could also replace the entry for "penguin_chief" in
that file. It still points to Linus Torvalds - but I assume he does not
like to be bothered with patches related to QEMU... ?
Thomas
[Qemu-devel] [PATCH] get_maintainer.pl: Remove the --git-chief-penguins option
Linus likely does not want to get e-mails about QEMU, so let's
just remove this option.
Suggested-by: Michael S. Tsirkin
Signed-off-by: Thomas Huth
---
scripts/get_maintainer.pl | 45 +
1 files changed, 1 insertions(+), 44 deletions(-)
diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index 38334de..4034997 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -23,7 +23,6 @@ my $email_usename = 1;
my $email_maintainer = 1;
my $email_list = 1;
my $email_subscriber_list = 0;
-my $email_git_penguin_chiefs = 0;
my $email_git = 0;
my $email_git_all_signature_types = 0;
my $email_git_blame = 0;
@@ -60,21 +59,6 @@ my $exit = 0;
my %commit_author_hash;
my %commit_signer_hash;
-my @penguin_chief = ();
-push(@penguin_chief, "Linus Torvalds:torvalds\@linux-foundation.org");
-#Andrew wants in on most everything - 2009/01/14
-#push(@penguin_chief, "Andrew Morton:akpm\@linux-foundation.org");
-
-my @penguin_chief_names = ();
-foreach my $chief (@penguin_chief) {
-if ($chief =~ m/^(.*):(.*)/) {
- my $chief_name = $1;
- my $chief_addr = $2;
- push(@penguin_chief_names, $chief_name);
-}
-}
-my $penguin_chiefs = "\(" . join("|", @penguin_chief_names) . "\)";
-
# Signature types of people who are either
# a) responsible for the code in question, or
# b) familiar enough with it to give relevant feedback
@@ -187,7 +171,6 @@ if (!GetOptions(
'git-blame!' => \$email_git_blame,
'git-blame-signatures!' => \$email_git_blame_signatures,
'git-fallback!' => \$email_git_fallback,
- 'git-chief-penguins!' => \$email_git_penguin_chiefs,
'git-min-signatures=i' => \$email_git_min_signatures,
'git-max-maintainers=i' => \$email_git_max_maintainers,
'git-min-percent=i' => \$email_git_min_percent,
@@ -256,7 +239,7 @@ if ($sections) {
if ($email &&
($email_maintainer + $email_list + $email_subscriber_list +
- $email_git + $email_git_penguin_chiefs + $email_git_blame) == 0) {
+ $email_git + $email_git_blame) == 0) {
die "$P: Please select at least 1 email option\n";
}
@@ -663,19 +646,6 @@ sub get_maintainers {
}
if ($email) {
- foreach my $chief (@penguin_chief) {
- if ($chief =~ m/^(.*):(.*)/) {
- my $email_address;
-
- $email_address = format_email($1, $2, $email_usename);
- if ($email_git_penguin_chiefs) {
- push(@email_to, [$email_address, 'chief penguin']);
- } else {
- @email_to = grep($_->[0] !~ /${email_address}/, @email_to);
- }
- }
- }
-
foreach my $email (@file_emails) {
my ($name, $address) = parse_email($email);
@@ -732,7 +702,6 @@ MAINTAINER field selection options:
--git-all-signature-types => include signers regardless of signature type
or use only ${signature_pattern} signers (default:
$email_git_all_signature_types)
--git-fallback => use git when no exact MAINTAINERS pattern (default:
$email_git_fallback)
---git-chief-penguins => include ${penguin_chiefs}
--git-min-signatures => number of signatures required (default:
$email_git_min_signatures)
--git-max-maintainers => maximum maintainers to add (default:
$email_git_max_maintainers)
--git-min-percent => minimum percentage of commits required (default:
$email_git_min_percent)
@@ -1273,10 +1242,6 @@ sub vcs_find_signers {
save_commits_by_author(@lines) if ($interactive);
save_commits_by_signer(@lines) if ($interactive);
-if (!$email_git_penguin_chiefs) {
- @signatures = grep(!/${penguin_chiefs}/i, @signatures);
-}
-
my ($types_ref, $signers_ref) = extract_formatted_signatures(@signatures);
return ($commits, @$signers_ref);
@@ -1288,10 +1253,6 @@ sub vcs_find_author {
@lines = &{$VCS_cmds{"execute_cmd"}}($cmd);
-if (!$email_git_penguin_chiefs) {
- @lines = grep(!/${penguin_chiefs}/i, @lines);
-}
-
return @lines if !@lines;
my @authors = ();
@@ -1917,10 +1878,6 @@ sub vcs_file_blame {
@lines = &{$VCS_cmds{"execute_cmd"}}($cmd);
- if (!$email_git_penguin_chiefs) {
- @lines = grep(!/${penguin_chiefs}/i, @lines);
- }
-
last if !@lines;
my @authors = ();
--
1.7.1
Re: [Qemu-devel] [PATCH 2/3 RFC] s390: implement pci instructions
Hi Frank,
On Wed, 22 Oct 2014 17:11:59 +0200
Frank Blaschka wrote:
> From: Frank Blaschka
>
> This patch implements the s390 pci instructions in qemu. It allows
> to access and drive pci devices attached to the s390 pci bus.
> Because of platform constrains devices using IO BARs are not
> supported. Also a device has to support MSI/MSI-X to run on s390.
>
> Signed-off-by: Frank Blaschka
> ---
> target-s390x/Makefile.objs | 2 +-
> target-s390x/kvm.c | 52
> target-s390x/pci_ic.c | 735
> +
> target-s390x/pci_ic.h | 316 +++
> 4 files changed, 1104 insertions(+), 1 deletion(-)
> create mode 100644 target-s390x/pci_ic.c
> create mode 100644 target-s390x/pci_ic.h
...
> diff --git a/target-s390x/pci_ic.c b/target-s390x/pci_ic.c
> new file mode 100644
> index 000..a496e6b
> --- /dev/null
> +++ b/target-s390x/pci_ic.c
> @@ -0,0 +1,735 @@
> +/*
> + * s390 PCI intercepts
> + *
> + * Copyright 2014 IBM Corp.
> + * Author(s): Frank Blaschka
> + *Hong Bo Li
> + *Yi Min Zhao
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or (at
> + * your option) any later version. See the COPYING file in the top-level
> + * directory.
> + */
> +
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "qemu-common.h"
> +#include "qemu/timer.h"
> +#include "migration/qemu-file.h"
> +#include "sysemu/sysemu.h"
> +#include "sysemu/kvm.h"
> +#include "cpu.h"
> +#include "sysemu/device_tree.h"
> +#include "monitor/monitor.h"
> +#include "pci_ic.h"
> +
> +#include "hw/hw.h"
> +#include "hw/pci/pci.h"
> +#include "hw/pci/pci_bridge.h"
> +#include "hw/pci/pci_bus.h"
> +#include "hw/pci/pci_host.h"
> +#include "hw/s390x/s390-pci-bus.h"
> +#include "exec/exec-all.h"
> +#include "exec/memory-internal.h"
> +
> +/* #define DEBUG_S390PCI_IC */
> +#ifdef DEBUG_S390PCI_IC
> +#define DPRINTF(fmt, ...) \
> +do { fprintf(stderr, "s390pci_ic: " fmt, ## __VA_ARGS__); } while (0)
> +#else
> +#define DPRINTF(fmt, ...) \
> +do { } while (0)
> +#endif
> +
> +static uint64_t resume_token;
> +
> +static uint8_t barsize(uint64_t size)
> +{
> +uint64_t mask = 1;
> +int i;
> +
> +if (!size) {
> +return 0;
> +}
> +
> +for (i = 0; i < 64; i++) {
> +if (size & mask) {
> +break;
> +}
> +mask = (mask << 1);
> +}
> +
> +return i;
> +}
> +
> +static void s390_set_status_code(CPUS390XState *env,
> + uint8_t r, uint64_t status_code)
> +{
> +env->regs[r] &= ~0xff00;
> +env->regs[r] |= (status_code & 0xff) << 24;
> +}
> +
> +static int list_pci(ClpReqRspListPci *rrb, uint8_t *cc)
> +{
> +S390PCIBusDevice *pbdev;
> +uint32_t res_code, initial_l2, g_l2, finish;
> +int rc, idx;
> +
> +rc = 0;
> +if (be16_to_cpu(rrb->request.hdr.len) != 32) {
> +res_code = CLP_RC_LEN;
> +rc = -EINVAL;
> +goto out;
> +}
> +
> +if ((be32_to_cpu(rrb->request.fmt) & CLP_MASK_FMT) != 0) {
> +res_code = CLP_RC_FMT;
> +rc = -EINVAL;
> +goto out;
> +}
> +
> +if ((be32_to_cpu(rrb->request.fmt) & ~CLP_MASK_FMT) != 0 ||
> +rrb->request.reserved1 != 0 ||
> +rrb->request.reserved2 != 0) {
> +res_code = CLP_RC_RESNOT0;
> +rc = -EINVAL;
> +goto out;
> +}
> +
> +if (be64_to_cpu(rrb->request.resume_token) == 0) {
> +resume_token = 0;
> +} else if (be64_to_cpu(rrb->request.resume_token) != resume_token) {
> +res_code = CLP_RC_LISTPCI_BADRT;
> +rc = -EINVAL;
> +goto out;
> +}
> +
> +if (be16_to_cpu(rrb->response.hdr.len) < 48) {
> +res_code = CLP_RC_8K;
> +rc = -EINVAL;
> +goto out;
> +}
> +
> +initial_l2 = be16_to_cpu(rrb->response.hdr.len);
> +if ((initial_l2 - LIST_PCI_HDR_LEN) % sizeof(ClpFhListEntry)
> +!= 0) {
> +rc = -EINVAL;
Don't you need to set res_code to a proper value here, too?
> +goto out;
> +}
> +
> +rrb->response.fmt = 0;
> +rrb->response.reserved1 = rrb->response.reserved2 = 0;
> +rrb->response.mdd = cpu_to_be32(FH_VIRT);
> +rrb->response.max_fn = cpu_to_be16(PCI_MAX_FUNCTIONS);
> +rrb->response.entry_size = sizeof(ClpFhListEntry);
> +finish = 0;
> +idx = resume_token;
> +g_l2 = LIST_PCI_HDR_LEN;
> +do {
> +pbdev = s390_pci_find_dev_by_idx(idx);
> +if (!pbdev) {
> +finish = 1;
> +break;
> +}
> +rrb->response.fh_list[idx - resume_token].device_id =
> +pci_get_word(pbdev->pdev->config + PCI_DEVICE_ID);
> +rrb->response.fh_list[idx - resume_token].vendor_id =
> +pci_get_word(pbdev->pdev->config + PCI_VENDOR_ID);
> +rrb->response.fh_list[idx - resume_token].config =
> +cpu_to_b
Re: [Qemu-devel] [PATCH v3 02/15] softmmu: provide softmmu access type enum
On Fri, 24 Oct 2014 13:42:16 +0100
Leon Alrae wrote:
> New MIPS features depend on the access type and enum is more convenient than
> using the numbers directly.
>
> Signed-off-by: Leon Alrae
> ---
> include/exec/cpu-common.h | 6 ++
> softmmu_template.h| 26 --
> 2 files changed, 22 insertions(+), 10 deletions(-)
>
> diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
> index e3ec4c8..427b851 100644
> --- a/include/exec/cpu-common.h
> +++ b/include/exec/cpu-common.h
> @@ -26,6 +26,12 @@ typedef struct CPUListState {
> FILE *file;
> } CPUListState;
>
> +typedef enum MMUAccessType {
> +MMU_DATA_LOAD = 0,
> +MMU_DATA_STORE = 1,
> +MMU_INST_FETCH = 2
> +} MMUAccessType;
> +
> #if !defined(CONFIG_USER_ONLY)
>
> enum device_endian {
> diff --git a/softmmu_template.h b/softmmu_template.h
> index 88e3390..6b4e615 100644
> --- a/softmmu_template.h
> +++ b/softmmu_template.h
> @@ -67,10 +67,10 @@
> #endif
>
> #ifdef SOFTMMU_CODE_ACCESS
> -#define READ_ACCESS_TYPE 2
> +#define READ_ACCESS_TYPE MMU_INST_FETCH
> #define ADDR_READ addr_code
> #else
> -#define READ_ACCESS_TYPE 0
> +#define READ_ACCESS_TYPE MMU_DATA_LOAD
> #define ADDR_READ addr_read
> #endif
>
> @@ -396,11 +396,12 @@ void helper_le_st_name(CPUArchState *env, target_ulong
> addr, DATA_TYPE val,
> != (tlb_addr & (TARGET_PAGE_MASK | TLB_INVALID_MASK))) {
> #ifdef ALIGNED_ONLY
> if ((addr & (DATA_SIZE - 1)) != 0) {
> -cpu_unaligned_access(ENV_GET_CPU(env), addr, 1, mmu_idx,
> retaddr);
> +cpu_unaligned_access(ENV_GET_CPU(env), addr, MMU_DATA_STORE,
> + mmu_idx, retaddr);
> }
> #endif
> if (!VICTIM_TLB_HIT(addr_write)) {
> -tlb_fill(ENV_GET_CPU(env), addr, 1, mmu_idx, retaddr);
> +tlb_fill(ENV_GET_CPU(env), addr, MMU_DATA_STORE, mmu_idx,
> retaddr);
> }
> tlb_addr = env->tlb_table[mmu_idx][index].addr_write;
> }
> @@ -427,7 +428,8 @@ void helper_le_st_name(CPUArchState *env, target_ulong
> addr, DATA_TYPE val,
> int i;
> do_unaligned_access:
> #ifdef ALIGNED_ONLY
> -cpu_unaligned_access(ENV_GET_CPU(env), addr, 1, mmu_idx, retaddr);
> +cpu_unaligned_access(ENV_GET_CPU(env), addr, MMU_DATA_STORE,
> + mmu_idx, retaddr);
> #endif
> /* XXX: not efficient, but simple */
> /* Note: relies on the fact that tlb_fill() does not remove the
> @@ -446,7 +448,8 @@ void helper_le_st_name(CPUArchState *env, target_ulong
> addr, DATA_TYPE val,
> /* Handle aligned access or unaligned access in the same page. */
> #ifdef ALIGNED_ONLY
> if ((addr & (DATA_SIZE - 1)) != 0) {
> -cpu_unaligned_access(ENV_GET_CPU(env), addr, 1, mmu_idx, retaddr);
> +cpu_unaligned_access(ENV_GET_CPU(env), addr, MMU_DATA_STORE,
> + mmu_idx, retaddr);
> }
> #endif
>
> @@ -474,11 +477,12 @@ void helper_be_st_name(CPUArchState *env, target_ulong
> addr, DATA_TYPE val,
> != (tlb_addr & (TARGET_PAGE_MASK | TLB_INVALID_MASK))) {
> #ifdef ALIGNED_ONLY
> if ((addr & (DATA_SIZE - 1)) != 0) {
> -cpu_unaligned_access(ENV_GET_CPU(env), addr, 1, mmu_idx,
> retaddr);
> +cpu_unaligned_access(ENV_GET_CPU(env), addr, MMU_DATA_STORE,
> + mmu_idx, retaddr);
> }
> #endif
> if (!VICTIM_TLB_HIT(addr_write)) {
> -tlb_fill(ENV_GET_CPU(env), addr, 1, mmu_idx, retaddr);
> +tlb_fill(ENV_GET_CPU(env), addr, MMU_DATA_STORE, mmu_idx,
> retaddr);
> }
> tlb_addr = env->tlb_table[mmu_idx][index].addr_write;
> }
> @@ -505,7 +509,8 @@ void helper_be_st_name(CPUArchState *env, target_ulong
> addr, DATA_TYPE val,
> int i;
> do_unaligned_access:
> #ifdef ALIGNED_ONLY
> -cpu_unaligned_access(ENV_GET_CPU(env), addr, 1, mmu_idx, retaddr);
> +cpu_unaligned_access(ENV_GET_CPU(env), addr, MMU_DATA_STORE,
> + mmu_idx, retaddr);
> #endif
> /* XXX: not efficient, but simple */
> /* Note: relies on the fact that tlb_fill() does not remove the
> @@ -524,7 +529,8 @@ void helper_be_st_name(CPUArchState *env, target_ulong
> addr, DATA_TYPE val,
> /* Handle aligned access or unaligned access in the same page. */
> #ifdef ALIGNED_ONLY
> if ((addr & (DATA_SIZE - 1)) != 0) {
> -cpu_unaligned_access(ENV_GET_CPU(env), addr, 1, mmu_idx, retaddr);
> +cpu_unaligned_access(ENV_GET_CPU(env), addr, MMU_DATA_STORE,
> + mmu_idx, retaddr);
> }
> #endif
>
I very much like the idea to get rid of these "magic" numbers!
Reviewed-by: Thomas Huth
Re: [Qemu-devel] [Qemu-ppc] [PATCH] pseries: Correct RAM size check for SLOF
Am Tue, 8 Nov 2011 16:52:16 +1100
schrieb David Gibson :
> The SLOF firmware used on the pseries machine needs a reasonable amount of
> (guest) RAM in order to run, so we have a check in the machine init
> function to check that this is available. However, SLOF runs in real mode
> (MMU off) which means it can only actually access the RMA (Real Mode Area),
> not all of RAM. In many cases the RMA is the same as all RAM, but when
> running with Book3S HV KVM on PowerPC 970, the RMA must be especially
> allocated to be (host) physically contiguous. In this case, the RMA size
> is determined by what the host admin allocated at boot time, and will
> usually be less than the whole guest RAM size.
>
> This patch corrects the test to see if SLOF has enough memory for this
> case.
[...]
> @@ -562,9 +562,9 @@ static void ppc_spapr_init(ram_addr_t ram_size,
>
> spapr->entry_point = KERNEL_LOAD_ADDR;
> } else {
> -if (ram_size < (MIN_RAM_SLOF << 20)) {
> +if (rma_size < (MIN_RMA_SLOF << 20)) {
> fprintf(stderr, "qemu: pSeries SLOF firmware requires >= "
> -"%ldM guest RAM\n", MIN_RAM_SLOF);
> +"%ldM guest RAM\n", MIN_RMA_SLOF);
Maybe it's more helpful for the unexperienced users when we change the
term "guest RAM" to "real mode area (RMA) memory" here? What do you
think?
Thomas
Re: [Qemu-devel] [Qemu-ppc] [PATCH 01/58] spapr: proper qdevification
Hi all! Am Fri, 16 Sep 2011 12:41:40 +0200 schrieb Paolo Bonzini : > On 09/16/2011 05:06 AM, David Gibson wrote: > >> > > >> > 1) do you mean, in Linux terms, one target per SCSI _host_ or one > >> > target per SCSI _channel_? i.e. if you looks at > >> > /sys/bus/scsi/devices, right now it looks like > >> > > >> > 0:0:0:00:0:1:0 (two targets on the same host and channel) > >> > > >> > Should it be? > >> > > >> > 0:0:0:00:1:0:0 (one target per channel) > >> > > >> > or > >> > > >> > 0:0:0:01:0:0:0 (one target per host) > >> > > >> > If it is the former, then you are simply hitting a limitation of the > >> > SCSI layer in QEMU and I do have patches to make assignment more > >> > flexible. Based on the Linux VSCSI driver, and based on what SLOF > >> > does, I'd guess that's what you mean. > > > > Well, now I'm confused. I had a look at a pHyp machine, and Linux > > seemed to see it as multiple targets on a single channel, but I'm sure > > the PAPR spec says you shouldn't have that. So I'm going to have to > > look closer now. > > If this is the case, there might be a bug in SLOF's probing of SCSI devices. > > SLOF probes target 0/LUN 0 on eight channels, i.e. from 0:0:0 to 7:0:0. > Linux however shows them the same as pHyp, i.e. from 0:0:0 to 0:7:0. > > The reason this works is because LUN parsing in QEMU is completely > broken (by Ben's admission in spapr_vscsi.c :)) and so SLOF's x:0:0 and > Linux's 0:x:0 end up referring to the same device. I've done some readings about this problem today, and I think I've got an idea what might be wrong here - seems like a bug in SLOF to me. First, according to the SLOF source code, it seems to me that its intention is to to scan target IDs, not channels (but as I haven't written that part, I am not 100% sure here). Then I compared how Linux and SLOF fill the 64-bit LUN field in the SRP_CMD request structure, and they both fill in the target ID at the same location - but Linux is additionally setting an additional bit in first byte (see the "lun_from_dev" function in ibmvscsi.c of the kernel). Looking at the "SCSI Architecture Model" specification, this additional bit is used to select the "Logical unit addressing method" instead of the "Peripheral device addressing method" that SLOF currently uses - and the "Logical unit addressing method" sounds more reasonable to me when looking at the places where SLOF tries to fill in the target ID. So I suggest that I change SLOF to also use the "Logical unit addressing method" like Linux does, which should result in the fact that SLOF tries to scan the target IDs instead of the channels/bus IDs. What do you think, does that sound ok? Regards, Thomas
Re: [Qemu-devel] [Qemu-ppc] [PATCH 01/58] spapr: proper qdevification
Am Fri, 16 Sep 2011 12:51:31 -0300
schrieb Benjamin Herrenschmidt :
>
> > I've done some readings about this problem today, and I think I've got
> > an idea what might be wrong here - seems like a bug in SLOF to me.
> >
> > First, according to the SLOF source code, it seems to me that its
> > intention is to to scan target IDs, not channels (but as I haven't
> > written that part, I am not 100% sure here).
> >
> > Then I compared how Linux and SLOF fill the 64-bit LUN field in the
> > SRP_CMD request structure, and they both fill in the target ID at the
> > same location - but Linux is additionally setting an additional bit in
> > first byte (see the "lun_from_dev" function in ibmvscsi.c of the
> > kernel).
> >
> > Looking at the "SCSI Architecture Model" specification, this additional
> > bit is used to select the "Logical unit addressing method" instead of
> > the "Peripheral device addressing method" that SLOF currently uses - and
> > the "Logical unit addressing method" sounds more reasonable to me when
> > looking at the places where SLOF tries to fill in the target ID.
> >
> > So I suggest that I change SLOF to also use the "Logical unit
> > addressing method" like Linux does, which should result in the fact that
> > SLOF tries to scan the target IDs instead of the channels/bus IDs.
>
> .../...
>
> Note that in addition to that, the PAPR spec specifies only one
> "device" (whatever that means) per vscsi instance.
Really? In that case, I wonder why Linux is using the "Logical unit
addressing format" with target IDs and bus numbers instead of the
"Flat space addressing method" for vscsi ... according to
drivers/scsi/ibmvscsi/ibmvscsi.c :
static inline u16 lun_from_dev(struct scsi_device *dev)
{
return (0x2 << 14) | (dev->id << 8) | (dev->channel << 5) | dev->lun;
}
In case there's really only one device per vscsi instance, shouldn't
that code use addressing method 0x1 instead of 0x2 here?
Thomas
[Qemu-devel] target-ppc: Problem with mtmsr emulation
Hi all! There seems to be a problem with the emulation of the mtmsr instruction: According to the PowerISA spec, chapter Book III-S, the mtmsr opcode has a so-called "L" field at bit position 15. Looking at the function gen_mtmsr() in target-ppc/translate.c, the bit is taken into account since the function checks for ctx->opcode & 0x0001. However, when looking at the GEN_HANDLER definition later in that file: GEN_HANDLER(mtmsr, 0x1F, 0x12, 0x04, 0x001FF801, PPC_MISC) you can see that the bit is marked as invalid bit in the 0x001FF801 mask, thus if the bit is set, a program exception is generated instead of executing the gen_mtmsr() function. An easy way to fix this for Book III-S is to change the mask to 0x001EF801 (just like the mask for mtmsrd), but I am afraid that this would break the Book III-E variant of mtmsr, since the embedded version does not have this bit defined. Any suggestions how to fix this problem in a proper way? Thomas
Re: [Qemu-devel] target-ppc: Problem with mtmsr emulation
On Fri, 28 Mar 2014 18:25:02 +0800 Alexander Graf wrote: > > > > Am 28.03.2014 um 16:16 schrieb Thomas Huth : > > > > > > Hi all! > > > > There seems to be a problem with the emulation of the mtmsr instruction: > > According to the PowerISA spec, chapter Book III-S, the mtmsr opcode > > has a so-called "L" field at bit position 15. Looking at the function > > gen_mtmsr() in target-ppc/translate.c, the bit is taken into account > > since the function checks for ctx->opcode & 0x0001. > > However, when looking at the GEN_HANDLER definition later in that file: > > > > GEN_HANDLER(mtmsr, 0x1F, 0x12, 0x04, 0x001FF801, PPC_MISC) > > > > you can see that the bit is marked as invalid bit in the 0x001FF801 > > mask, thus if the bit is set, a program exception is generated instead > > of executing the gen_mtmsr() function. > > > > An easy way to fix this for Book III-S is to change the mask to > > 0x001EF801 (just like the mask for mtmsrd), but I am afraid that this > > would break the Book III-E variant of mtmsr, since the embedded version > > does not have this bit defined. Any suggestions how to fix this problem > > in a proper way? > > Please check in the older isa versions whether that bit is declared reserved. > > If it is, we need to make sure we only match it on newer ISA conformance. The oldest ISA version that I've found (version 2.01, from 2003) already contains the L bit, so I assume it's always been there. So it's likely just a Book III-S vs. Book III-E issue. Thomas
Re: [Qemu-devel] [PATCH v6 2/8] virtio: allow byte swapping for vring and config access
total_bufs = num_bufs;
> @@ -459,15 +470,15 @@ int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem)
> vring_avail_event(vq, vring_avail_idx(vq));
> }
>
> -if (vring_desc_flags(desc_pa, i) & VRING_DESC_F_INDIRECT) {
> -if (vring_desc_len(desc_pa, i) % sizeof(VRingDesc)) {
> +if (vring_desc_flags(desc_pa, i, vq->vdev) & VRING_DESC_F_INDIRECT) {
> +if (vring_desc_len(desc_pa, i, vq->vdev) % sizeof(VRingDesc)) {
> error_report("Invalid size for indirect buffer table");
> exit(1);
> }
>
> /* loop over the indirect descriptor table */
> -max = vring_desc_len(desc_pa, i) / sizeof(VRingDesc);
> -desc_pa = vring_desc_addr(desc_pa, i);
> +max = vring_desc_len(desc_pa, i, vq->vdev) / sizeof(VRingDesc);
> +desc_pa = vring_desc_addr(desc_pa, i, vq->vdev);
> i = 0;
> }
>
> @@ -475,30 +486,32 @@ int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem)
> do {
> struct iovec *sg;
>
> -if (vring_desc_flags(desc_pa, i) & VRING_DESC_F_WRITE) {
> +if (vring_desc_flags(desc_pa, i, vq->vdev) & VRING_DESC_F_WRITE) {
> if (elem->in_num >= ARRAY_SIZE(elem->in_sg)) {
> error_report("Too many write descriptors in indirect table");
> exit(1);
> }
> -elem->in_addr[elem->in_num] = vring_desc_addr(desc_pa, i);
> +elem->in_addr[elem->in_num] = vring_desc_addr(desc_pa, i,
> + vq->vdev);
That one seems to be pretty close, but could also still fit into 80
columns?
> sg = &elem->in_sg[elem->in_num++];
> } else {
> if (elem->out_num >= ARRAY_SIZE(elem->out_sg)) {
> error_report("Too many read descriptors in indirect table");
> exit(1);
> }
> -elem->out_addr[elem->out_num] = vring_desc_addr(desc_pa, i);
> +elem->out_addr[elem->out_num] = vring_desc_addr(desc_pa, i,
> +vq->vdev);
> sg = &elem->out_sg[elem->out_num++];
> }
>
> -sg->iov_len = vring_desc_len(desc_pa, i);
> +sg->iov_len = vring_desc_len(desc_pa, i, vq->vdev);
>
> /* If we've got too many, that implies a descriptor loop. */
> if ((elem->in_num + elem->out_num) > max) {
> error_report("Looped descriptor");
> exit(1);
> }
> -} while ((i = virtqueue_next_desc(desc_pa, i, max)) != max);
> +} while ((i = virtqueue_next_desc(desc_pa, i, max, vq->vdev)) != max);
>
> /* Now map what we have collected */
> virtqueue_map_sg(elem->in_sg, elem->in_addr, elem->in_num, 1);
Apart from the cosmetic nits, patch looks fine to me.
Reviewed-by: Thomas Huth
Re: [Qemu-devel] [PATCH v6 1/8] virtio_legacy_get_byteswap: endian-ambivalent targets using legacy virtio
On Fri, 28 Mar 2014 11:57:17 +0100
Greg Kurz wrote:
> From: Rusty Russell
>
> virtio data structures are defined as "target endian", which assumes
> that's a fixed value. In fact, that actually means it's platform-specific.
> The OASIS virtio 1.0 spec will fix this, by making all little endian.
>
> We need to support both implementations and we want to share as much code
> as possible.
>
> A good way to do it is to introduce a per-device boolean property to tell
> memory accessors whether they should swap bytes or not. This flag should
> be set at device reset time, because:
> - endianness won't change while the device is in use, and if we reboot
> into a different endianness, a new device reset will occur
> - as suggested by Alexander Graf, we can keep all the logic to set the
> property in a single place and share all the virtio memory accessors
> between the two implementations
>
> For legacy devices, we rely on a per-platform hook to set the flag. The
> virtio 1.0 implementation will just have to add some more logic in
> virtio_reset() instead of calling the hook:
>
> if (vdev->legacy) {
>vdev->needs_byteswap = virtio_legacy_get_byteswap();
> } else {
> #ifdef HOST_WORDS_BIGENDIAN
>vdev->needs_byteswap = true;
> #else
>vdev->needs_byteswap = false;
> #endif
> }
>
> The needs_byteswap flag is preserved accross migrations.
>
> Signed-off-by: Rusty Russell
> [ fixed checkpatch.pl error with the virtio_byteswap initialisation,
> ldq_phys() API change,
> relicensed virtio-access.h to GPLv2+ on Rusty's request,
> introduce a per-device needs_byteswap flag,
> add VirtIODevice * arg to virtio helpers,
> rename virtio_get_byteswap to virtio_legacy_get_byteswap,
> Greg Kurz ]
> Signed-off-by: Greg Kurz
> ---
> hw/virtio/virtio.c|5 +
> include/hw/virtio/virtio-access.h | 139
> +
> include/hw/virtio/virtio.h|3 +
> stubs/Makefile.objs |1
> stubs/virtio_get_byteswap.c |6 ++
> 5 files changed, 154 insertions(+)
> create mode 100644 include/hw/virtio/virtio-access.h
> create mode 100644 stubs/virtio_get_byteswap.c
>
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index aeabf3a..24b565f 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -19,6 +19,7 @@
> #include "hw/virtio/virtio.h"
> #include "qemu/atomic.h"
> #include "hw/virtio/virtio-bus.h"
> +#include "hw/virtio/virtio-access.h"
>
> /*
> * The alignment to use between consumer and producer parts of vring.
> @@ -546,6 +547,8 @@ void virtio_reset(void *opaque)
>
> virtio_set_status(vdev, 0);
>
> +vdev->needs_byteswap = virtio_legacy_get_byteswap();
> +
> if (k->reset) {
> k->reset(vdev);
> }
> @@ -845,6 +848,7 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)
>
> qemu_put_8s(f, &vdev->status);
> qemu_put_8s(f, &vdev->isr);
> +qemu_put_8s(f, (uint8_t *) &vdev->needs_byteswap);
> qemu_put_be16s(f, &vdev->queue_sel);
> qemu_put_be32s(f, &vdev->guest_features);
> qemu_put_be32(f, vdev->config_len);
> @@ -905,6 +909,7 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f)
>
> qemu_get_8s(f, &vdev->status);
> qemu_get_8s(f, &vdev->isr);
> +qemu_get_8s(f, (uint8_t *) &vdev->needs_byteswap);
> qemu_get_be16s(f, &vdev->queue_sel);
> qemu_get_be32s(f, &features);
Two remarks here:
1) You've declared needs_byteswap as "bool", but in above code you
assume that the "bool" type is implemented as a single byte. That's
most likely true on most system, but AFAIK it's specific to the
compiler. So for portable code, I think you should either change the
type of needs_byteswap or you should use a temporary uint8_t variable
here instead.
2) You're changing the layout of the saved data ... don't you also have
to increase the version numbers in that case, too? (e.g. change the
version id for the register_savevm call in virtio-blk.c, virtio-net.c,
etc.)?
Thomas
Re: [Qemu-devel] [PATCH 1/5] s390x/cpu: Use ioctl to reset state in the kernel
On Mon, 14 Apr 2014 12:14:49 +0200
Alexander Graf wrote:
>
> On 26.02.14 12:38, Christian Borntraeger wrote:
> > From: Thomas Huth
> >
> > Some of the state in the kernel can not be reset from QEMU yet.
> > For this we've got to use the KVM_S390_INITIAL_RESET ioctl to make
> > sure that the state in the kernel is set to the right values during
> > initial CPU reset, too.
> >
> > Signed-off-by: Thomas Huth
> > Acked-by: Cornelia Huck
> > Signed-off-by: Christian Borntraeger
> > ---
> > target-s390x/cpu.c | 9 +
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> > index f1319e5..1a8c1cc 100644
> > --- a/target-s390x/cpu.c
> > +++ b/target-s390x/cpu.c
> > @@ -108,6 +108,15 @@ static void s390_cpu_initial_reset(CPUState *s)
> > env->cregs[14] = CR14_RESET;
> >
> > env->pfault_token = -1UL;
> > +
> > +#if defined(CONFIG_KVM)
> > +/* Reset state inside the kernel that we cannot access yet from QEMU.
> > */
> > +if (kvm_enabled()) {
> > +if (kvm_vcpu_ioctl(s, KVM_S390_INITIAL_RESET, NULL)) {
>
> Could we put this into the vcpu register sync function? It gets a
> parameter that indicates when we are on RESET level, right?
No, sorry, as far as I can see, this is not that easily possible:
On S390, we've got five different levels of reset (CPU reset,
Initial CPU reset, Subsystem reset, Clear reset & Power-on reset).
The ioctl is about initial CPU reset only, while the register sync
function flag is rather only used by system/clear reset instead.
So for example when the guest OS sends a SIGP INITIAL CPU RESET from
one CPU to another, only the s390_cpu_initial_reset() will be called,
but not the register sync with KVM_PUT_RESET_STATE.
Thomas
Re: [Qemu-devel] [PATCH 0/7] virtio endian-ambivalent target fixes.
On Fri, 14 Feb 2014 10:38:02 +0100 Greg Kurz wrote: > On Thu, 17 Oct 2013 14:23:35 +1030 > Rusty Russell wrote: > > This is a re-transmit of the core of the virtio endian code. Since > > there seems to be some interest in ARM BE virtio, I've separated this from > > the direct problem I was solving: PowerPC LE. > > > > Please apply! > > Rusty. > > > > Hi, > > This serie is needed to enable current legacy virtio devices in a > cross-endian environment. Even though virtio-1.0 will address endianess > questions at the specification level, it is still in its early boot phase > and no code will be available before long (Rusty, please correct me if I am > wrong). > > We have all the PPC KVM bits in 3.14 already. We have PPC QEMU patches > ready to be applied by Alexander, as soon as the common code gets in. > Anthony has already positively reviewed this serie. We have been testing > for some monthes... Now we are waiting for partners ! :) > > Are the ARM people still interested in cross-endian virtio ? Are there > other people interested (Thomas for s390) ? FYI: s390 is always big-endian, so we currently do not have a problem yet. However, as soon as the Virtio-1.0 standard materializes, we will have to swap the ring data on s390, since it is then changed to little-endian. That's why we are interested in this patch series, too. So Rusty, if you (or somebody else) could post an updated version of this patch series, that would be great! Regards, Thomas
Re: [Qemu-devel] Buildbot failure: MinGW
On Wed, 19 Feb 2014 11:53:09 +
Peter Maydell wrote:
> On 19 February 2014 04:27, Stefan Weil wrote:
> > Hi Gerd, hi Stefan,
> >
> > we now need a C++ compiler on the buildbots. Currently, it's missing for
> > MinGW:
> >
> > /bin/sh: i686-pc-mingw32-g++: command not found
>
> Don't we successfully fall back to "don't build C++ things" if
> configure doesn't detect the C++ compiler?
I recently had a similar problem compiling QEMU on a freshly installed
system, where I only had a normal C compiler, but no C++ installed yet.
In rules.mak, you can find these lines:
# If we have a CXX we might have some C++ objects, in which case we
# must link with the C++ compiler, not the plain C compiler.
LINKPROG = $(or $(CXX),$(CC))
So that's ok, it sets LINKPROG to the c++ compiler if the variable is
set, and to the C compiler if not.
But now have a look at the configure script:
if test -z "${CXX}${cross_prefix}"; then
cxx="c++"
else
cxx="${CXX-${cross_prefix}g++}"
fi
[...]
echo "CXX=$cxx" >> $config_host_mak
That seems to always set the CXX variable! I think the above
if-statement is wrong, it should set cxx only if the c++ program is
really available.
Thomas
Re: [Qemu-devel] [PATCH 00/28] target-ppc: Altivec 2.07
On Thu, 20 Feb 2014 12:34:00 + "Richard W.M. Jones" wrote: [...] > > The second bug is kind of interesting. If you add ~ 256 disks (using > virtio-scsi), then it looks as if the firmware crashes. The total > console output is below. It looks as if "c >" is some kind of prompt. Yes, that's the Open Firmware prompt of the SLOF firmware here. If you know how to program in Forth, you can use it to debug the system. > Populating /vdevice methods > Populating /vdevice/vty@3000 > Populating /vdevice/nvram@7100 > C0580\b\b\b\b\bC05A0\b\b\b\b\bPopulating /pci@8002000 > ^M Adapters on 08002000 > 00 (D) : 106b 003fserial bus [ usb-ohci ] > 00 0800 (D) : 1af4 1004virtio [ scsi ] > Populating /pci@8002000/scsi@1 >SCSI: Looking for devices > 100 DISK : "QEMU QEMU HARDDISK1.7." > 101 DISK : "QEMU QEMU HARDDISK1.7." > 102 DISK : "QEMU QEMU HARDDISK1.7." > 103 DISK : "QEMU QEMU HARDDISK1.7." > 104 DISK : "QEMU QEMU HARDDISK1.7." > 105 DISK : "QEMU QEMU HARDDISK1.7." > 106 DISK : "QEMU QEMU HARDDISK1.7." > 107 DISK : "QEMU QEMU HARDDISK1.7." > 108 DISK : "QEMU QEMU HARDDISK1.7." > C0600\b\b\b\b\bC0640\b\b\b\b\bC0690\b\b\b\b\bC06A0\b\b\b\b\bC06A8\b\b\b\b\bC06B0\b\b\b\b\bC06B8\b\b\b\b\bC06C0\b\b\b\b\b > So the last successful checkpoint seems to be C06C0 ? Looking at https://github.com/aik/SLOF/blob/master/board-qemu/slof/tree.fs there is not much critical stuff going on between that checkpoint and the next one, so I guess the firmware just ran out of internal memory. When you run into this crash again, could you try to enter the following commands? They might help to confirm my suspicion: .free .available here . Please also say how much memory you were using for the guest. Thomas
Re: [Qemu-devel] [PATCH] configure: check that C++ compiler actually works
On Thu, 20 Feb 2014 15:10:16 +
Peter Maydell wrote:
> Check that the C++ compiler works with the C compiler; if it
> does not, then don't pass CXX to the build process. This
> fixes a regression where QEMU was no longer building if the
> build environment didn't have a C++ compiler (introduced
> in commit 3144f78b, which incorrectly assumed that rules.mak
> would only see a non-empty $(CXX) if configure had actually
> found a working C++ compiler).
>
> Signed-off-by: Peter Maydell
> Reported-by: Alexey Kardashevskiy
> Reported-by: Thomas Huth
> ---
> Apologies for the breakage for people who were building in
> setups with no C++ compiler -- I should have tested the
> original change more thoroughly.
>
> configure | 29 +
> 1 file changed, 29 insertions(+)
>
> diff --git a/configure b/configure
> index 4648117..6829cbb 100755
> --- a/configure
> +++ b/configure
> @@ -1289,6 +1289,35 @@ else
> error_exit "\"$cc\" either does not exist or does not work"
> fi
>
> +# Check that the C++ compiler exists and works with the C compiler
> +if has $cxx; then
> +cat > $TMPC < +int c_function(void);
> +int main(void) { return c_function(); }
> +EOF
> +
> +compile_object
> +
> +cat > $TMPC < +extern "C" {
> + int c_function(void);
> +}
> +int c_function(void) { return 42; }
> +EOF
> +
> +if (cc=$cxx do_cc $QEMU_CFLAGS -o $TMPE $TMPC $TMPO $LDFLAGS); then
> +# C++ compiler $cxx works ok with C compiler $cc
> +:
> +else
> +echo "C++ compiler $cxx does not work with C compiler $cc"
> +echo "Disabling C++ specific optional code"
> +cxx=
> +fi
> +else
> +echo "No C++ compiler available; disabling C++ specific optional code"
> +cxx=
> +fi
> +
> # Consult white-list to determine whether to enable werror
> # by default. Only enable by default for git builds
> z_version=`cut -f3 -d. $source_path/VERSION`
Works fine for me now on my system w/o C++ compiler:
Tested-by: Thomas Huth
Re: [Qemu-devel] [PATCH] configure: Make C++ test work with --enable-werror
> +else
> +werror="no"
> +fi
> +fi
> +
> # check that the C compiler works.
> cat > $TMPC < int main(void) { return 0; }
> @@ -1347,7 +1388,9 @@ extern "C" {
> int c_function(void) { return 42; }
> EOF
>
> -if (cc=$cxx do_cc $QEMU_CFLAGS -o $TMPE $TMPC $TMPO $LDFLAGS); then
> +update_cxxflags
> +
> +if do_cxx $QEMU_CXXFLAGS -o $TMPE $TMPC $TMPO $LDFLAGS; then
> # C++ compiler $cxx works ok with C compiler $cc
> :
> else
> @@ -1360,19 +1403,6 @@ else
> cxx=
> fi
>
> -# Consult white-list to determine whether to enable werror
> -# by default. Only enable by default for git builds
> -z_version=`cut -f3 -d. $source_path/VERSION`
> -
> -if test -z "$werror" ; then
> -if test -d "$source_path/.git" -a \
> -"$linux" = "yes" ; then
> -werror="yes"
> -else
> -werror="no"
> -fi
> -fi
> -
> gcc_flags="-Wold-style-declaration -Wold-style-definition -Wtype-limits"
> gcc_flags="-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers
> $gcc_flags"
> gcc_flags="-Wmissing-include-dirs -Wempty-body -Wnested-externs $gcc_flags"
Looks fine to me.
Reviewed-by: Thomas Huth
Re: [Qemu-devel] [PATCH v5 0/2] target-ppc: CPU device tree id
On Fri, 31 Jan 2014 17:07:36 +1100 Alexey Kardashevskiy wrote: > On 01/22/2014 03:20 PM, Alexey Kardashevskiy wrote: > > On 01/10/2014 07:20 PM, Alexey Kardashevskiy wrote: > >> On 12/20/2013 12:16 AM, Alexey Kardashevskiy wrote: > >>> On 12/10/2013 07:16 PM, Alexey Kardashevskiy wrote: > On 12/03/2013 02:30 PM, Alexey Kardashevskiy wrote: > > Hi! > > > > This is some cleanup. Please, comment. Thanks! > > > Ping, anyone? > >>> > >>> > >>> Ping? > >> > >> > >> > >> Ping, anyone? > > > > Ping, anyone? > > > Please, anybody, say something. Maybe poke the maintainers on IRC? Thomas
Re: [Qemu-devel] [PATCH 01/22] block: Move initialisation of BlockLimits to bdrv_refresh_limits()
On Thu, 12 Dec 2013 10:57:49 +0800
Wenchao Xia wrote:
>
> >> +static int bdrv_refresh_limits(BlockDriverState *bs)
> >> +{
> >> +BlockDriver *drv = bs->drv;
> >> +
> >> +memset(&bs->bl, 0, sizeof(bs->bl));
> >> +
> >> +if (!drv) {
> >> +return 0;
> >> +} else if (drv->bdrv_refresh_limits) {
> >> +return drv->bdrv_refresh_limits(bs);
> >> +}
> >> +
> >> +return 0;
> > It seems this line can be removed.
> >
> I missed the "else if", then the patch is OK.
But it could also be written in a shorter way:
if (drv && drv->bdrv_refresh_limits) {
return drv->bdrv_refresh_limits(bs);
}
return 0;
Regards,
Thomas
Re: [Qemu-devel] [PATCH 5/8] s390x/kvm: Implemented SIGP START
Hi Alex,
On Tue, 17 Dec 2013 14:56:33 +0100
Alexander Graf wrote:
>
> On 17.12.2013, at 14:22, Jens Freimann wrote:
>
> > From: Thomas Huth
> >
> > This patch adds the missing START order to the SIGP instruction handler.
>
> Does the spec define what happens on START when the CPU is already running?
As far as I can see, the spec does not explicitely defines the behavior
in that case, it says only: "The order is effective only when the
addressed CPU is in the stopped state". But according to my experiments
(I wrote a test program that I ran without additional hypervisor), the
START order is simply ignored when the CPU is already running and CC0 is
returned.
This is also what happens with the code below, since
s390_add_running_cpu() only does something if the "halted" flag is set,
and kicking a CPU that is already running does not hurt either, as far
as I can see.
> Does START modify any register state?
No, the CPU simply continues running with the state where it has been
stopped before.
Thomas
>
> >
> > Signed-off-by: Thomas Huth
> > Reviewed-by: Cornelia Huck
> > Signed-off-by: Jens Freimann
> > ---
> > target-s390x/kvm.c | 11 +++
> > 1 file changed, 11 insertions(+)
> >
> > diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
> > index 8c54134..fcc159f 100644
> > --- a/target-s390x/kvm.c
> > +++ b/target-s390x/kvm.c
> > @@ -588,6 +588,14 @@ static int handle_diag(S390CPU *cpu, struct kvm_run
> > *run, uint32_t ipb)
> > return r;
> > }
> >
> > +static int kvm_s390_cpu_start(S390CPU *cpu)
> > +{
> > +s390_add_running_cpu(cpu);
> > +qemu_cpu_kick(CPU(cpu));
> > +DPRINTF("DONE: KVM cpu start: %p\n", &cpu->env);
> > +return 0;
> > +}
> > +
> > int kvm_s390_cpu_restart(S390CPU *cpu)
> > {
> > kvm_s390_interrupt(cpu, KVM_S390_RESTART, 0);
> > @@ -642,6 +650,9 @@ static int handle_sigp(S390CPU *cpu, struct kvm_run
> > *run, uint8_t ipa1)
> > }
> >
> > switch (order_code) {
> > +case SIGP_START:
> > +r = kvm_s390_cpu_start(target_cpu);
> > +break;
> > case SIGP_RESTART:
> > r = kvm_s390_cpu_restart(target_cpu);
> > break;
> > --
> > 1.8.3.4
> >
>
Re: [Qemu-devel] [PATCH v2] spapr: make sure RMA is in first mode of first memory node
On Wed, 6 Nov 2013 18:54:11 +1100
Alexey Kardashevskiy wrote:
> The SPAPR specification says that the RMA starts at the LPAR's logical
> address 0 and is the first logical memory block reported in
> the LPAR’s device tree.
>
> So SLOF only maps the first block and that block needs to span
> the full RMA.
>
> This makes sure that the RMA area is where SLOF expects it.
>
> Cc: Benjamin Herrenschmidt
> Signed-off-by: Alexey Kardashevskiy
> ---
>
> I came up with v1 of the patch but the actual code came from
> Alexander Graf. Who should I put as author of this?
>
> ---
> hw/ppc/spapr.c | 9 -
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 57b38cf..57473df 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1114,6 +1114,7 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
> MemoryRegion *sysmem = get_system_memory();
> MemoryRegion *ram = g_new(MemoryRegion, 1);
> hwaddr rma_alloc_size;
> +hwaddr node0_size = (nb_numa_nodes > 1) ? node_mem[0] : ram_size;
> uint32_t initrd_base = 0;
> long kernel_size = 0, initrd_size = 0;
> long load_limit, rtas_limit, fw_size;
> @@ -1138,7 +1139,7 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
> if (rma_alloc_size && (rma_alloc_size < ram_size)) {
> spapr->rma_size = rma_alloc_size;
> } else {
> -spapr->rma_size = ram_size;
> +spapr->rma_size = node0_size;
>
> /* With KVM, we don't actually know whether KVM supports an
> * unbounded RMA (PR KVM) or is limited by the hash table size
> @@ -1155,6 +1156,12 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
> }
> }
>
> +if (spapr->rma_size > node0_size) {
> +fprintf(stderr, "Error: Numa node 0 has to span the RMA
> (%#08"HWADDR_PRIx")\n",
> +spapr->rma_size);
> +exit(1);
> +}
> +
What about the "if (spapr->rma_size > node0_size)" at the beginning of
spapr_populate_memory()? Could/should that go away now?
Thomas
Re: [Qemu-devel] [PATCH v3] spapr: make sure RMA is in first mode of first memory node
On Thu, 7 Nov 2013 14:50:00 +1100
Alexey Kardashevskiy wrote:
> The SPAPR specification says that the RMA starts at the LPAR's logical
> address 0 and is the first logical memory block reported in
> the LPAR’s device tree.
>
> So SLOF only maps the first block and that block needs to span
> the full RMA.
>
> This makes sure that the RMA area is where SLOF expects it.
>
> Cc: Thomas Huth
> Cc: Benjamin Herrenschmidt
> Signed-off-by: Alexey Kardashevskiy
> ---
> Changes:
> v3:
> * removed unnecessary RMA fixup from spapr_populate_memory()
>
> v2:
> * changed as recommended by Alex Graf
> ---
> hw/ppc/spapr.c | 12
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 7e53a5f..1f320f6 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -532,9 +532,6 @@ static int spapr_populate_memory(sPAPREnvironment *spapr,
> void *fdt)
>
> /* memory node(s) */
> node0_size = (nb_numa_nodes > 1) ? node_mem[0] : ram_size;
> -if (spapr->rma_size > node0_size) {
> -spapr->rma_size = node0_size;
> -}
>
> /* RMA */
> mem_reg_property[0] = 0;
> @@ -1113,6 +1110,7 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
> MemoryRegion *sysmem = get_system_memory();
> MemoryRegion *ram = g_new(MemoryRegion, 1);
> hwaddr rma_alloc_size;
> +hwaddr node0_size = (nb_numa_nodes > 1) ? node_mem[0] : ram_size;
> uint32_t initrd_base = 0;
> long kernel_size = 0, initrd_size = 0;
> long load_limit, rtas_limit, fw_size;
> @@ -1137,7 +1135,7 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
> if (rma_alloc_size && (rma_alloc_size < ram_size)) {
Uh, sorry for not seeing this earlier, ... but should that be
"rma_alloc_size < node0_size" instead of "rma_alloc_size < ram_size"?
For example, if rma_alloc_size = 256 MB, ram_size = 512 MB but
node0_size = 128 MB, this will set rma_size to 256 MB ...
> spapr->rma_size = rma_alloc_size;
> } else {
> -spapr->rma_size = ram_size;
> +spapr->rma_size = node0_size;
>
> /* With KVM, we don't actually know whether KVM supports an
> * unbounded RMA (PR KVM) or is limited by the hash table size
> @@ -1154,6 +1152,12 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
> }
> }
>
> +if (spapr->rma_size > node0_size) {
> +fprintf(stderr, "Error: Numa node 0 has to span the RMA
> (%#08"HWADDR_PRIx")\n",
> +spapr->rma_size);
> +exit(1);
> +}
... so in the above example you would get the error + exit here.
Or did I miss something?
Thomas
Re: [Qemu-devel] [PATCH v4] spapr: make sure RMA is in first mode of first memory node
On Fri, 8 Nov 2013 14:27:40 +1100
Alexey Kardashevskiy wrote:
> The SPAPR specification says that the RMA starts at the LPAR's logical
> address 0 and is the first logical memory block reported in
> the LPAR’s device tree.
>
> So SLOF only maps the first block and that block needs to span
> the full RMA.
>
> This makes sure that the RMA area is where SLOF expects it.
>
> Cc: Thomas Huth
> Cc: Benjamin Herrenschmidt
> Signed-off-by: Alexey Kardashevskiy
> ---
> Changes:
> v4:
> * fixed a bug with preallocated RMA (thanks to Thomas Huth)
>
> v3:
> * removed unnecessary RMA fixup from spapr_populate_memory()
>
> v2:
> * changed as recommended by Alex Graf
> ---
> hw/ppc/spapr.c | 14 +-
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 7e53a5f..036246c 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -532,9 +532,6 @@ static int spapr_populate_memory(sPAPREnvironment *spapr,
> void *fdt)
>
> /* memory node(s) */
> node0_size = (nb_numa_nodes > 1) ? node_mem[0] : ram_size;
> -if (spapr->rma_size > node0_size) {
> -spapr->rma_size = node0_size;
> -}
>
> /* RMA */
> mem_reg_property[0] = 0;
> @@ -1113,6 +1110,7 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
> MemoryRegion *sysmem = get_system_memory();
> MemoryRegion *ram = g_new(MemoryRegion, 1);
> hwaddr rma_alloc_size;
> +hwaddr node0_size = (nb_numa_nodes > 1) ? node_mem[0] : ram_size;
> uint32_t initrd_base = 0;
> long kernel_size = 0, initrd_size = 0;
> long load_limit, rtas_limit, fw_size;
> @@ -1134,10 +1132,10 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
> exit(1);
> }
>
> -if (rma_alloc_size && (rma_alloc_size < ram_size)) {
> +if (rma_alloc_size && (rma_alloc_size < node0_size)) {
> spapr->rma_size = rma_alloc_size;
> } else {
> -spapr->rma_size = ram_size;
> +spapr->rma_size = node0_size;
>
> /* With KVM, we don't actually know whether KVM supports an
> * unbounded RMA (PR KVM) or is limited by the hash table size
> @@ -1154,6 +1152,12 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
> }
> }
>
> +if (spapr->rma_size > node0_size) {
> +fprintf(stderr, "Error: Numa node 0 has to span the RMA
> (%#08"HWADDR_PRIx")\n",
> +spapr->rma_size);
> +exit(1);
> +}
> +
> /* We place the device tree and RTAS just below either the top of the
> RMA,
> * or just below 2GB, whichever is lowere, so that it can be
> * processed with 32-bit real mode code if necessary */
Looks fine to me now!
Reviewed-by: Thomas Huth
Re: [Qemu-devel] [PATCH 0/7] virtio endian-ambivalent target fixes.
On Thu, 17 Oct 2013 14:23:35 +1030 Rusty Russell wrote: > This is a re-transmit of the core of the virtio endian code. Since > there seems to be some interest in ARM BE virtio, I've separated this from > the direct problem I was solving: PowerPC LE. > > Please apply! > Rusty. > > Rusty Russell (7): > virtio_get_byteswap: function for endian-ambivalent targets using > virtio. > virtio: allow byte swapping for vring and config access > hw/net/virtio-net: use virtio wrappers to access headers. > hw/net/virtio-balloon: use virtio wrappers to access page frame > numbers. > hw/block/virtio-blk: use virtio wrappers to access headers. > hw/scsi/virtio-scsi: use virtio wrappers to access headers. > hw/char/virtio-serial-bus: use virtio wrappers to access headers. Hi Rusty! May I ask what's the current status of your virtio endian patches? We likely need something similar when we enable Virtio v1.0 for S390 virtio-ccw since we then have to byteswap the virtio stuff there, too. So I recently started to have a look at this... However, in your patches, the byteswapping seems to be activated/disabled globally, with the "virtio_byteswap" variable. But with Virtio v1.0, the guest can decide on a per-device basis whether it wants to drive the device in v1.0 mode (--> byteswap on S390) or in v0.9 legacy mode (--> no byteswap), depending on whether it sets the VIRTIO_F_VERSION_1 feature bit or not. I guess other architectures will have the same problem with Virtio 1.0, too, when the guests are not running in little endian mode. So I wonder whether it would it be feasible to change the code so that the decision of byteswapping or not is done on a per-device basis instead? What do you think? Regards, Thomas
Re: [Qemu-devel] [PATCH v5 2/2] spapr: limit numa memory regions by ram size
On Mon, 18 Nov 2013 15:19:32 +1100
Alexey Kardashevskiy wrote:
> From: Paul Mackerras
>
> This makes sure that all NUMA memory blocks beside within RAM or
s/beside/reside/ ?
> have zero length.
>
> Signed-off-by: Alexey Kardashevskiy
> ---
>
> This is a bugfix for:
> -m 500
> -smp 8,sockets=2,cores=2,threads=2
> -numa node,nodeid=0,cpus=0-3,mem=500
> -numa node,nodeid=1,cpus=4-7,mem=500
> ---
> hw/ppc/spapr.c | 20
> 1 file changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 036246c..a7f6af8 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -526,12 +526,16 @@ static int spapr_populate_memory(sPAPREnvironment
> *spapr, void *fdt)
> cpu_to_be32(0x0), cpu_to_be32(0x0),
> cpu_to_be32(0x0)};
> char mem_name[32];
> -hwaddr node0_size, mem_start;
> +hwaddr node0_size, mem_start, node_size;
> uint64_t mem_reg_property[2];
> int i, off;
>
> /* memory node(s) */
> -node0_size = (nb_numa_nodes > 1) ? node_mem[0] : ram_size;
> +if (nb_numa_nodes > 1 && node_mem[0] < ram_size) {
> +node0_size = node_mem[0];
> +} else {
> +node0_size = ram_size;
> +}
>
> /* RMA */
> mem_reg_property[0] = 0;
> @@ -563,7 +567,15 @@ static int spapr_populate_memory(sPAPREnvironment
> *spapr, void *fdt)
> mem_start = node0_size;
> for (i = 1; i < nb_numa_nodes; i++) {
> mem_reg_property[0] = cpu_to_be64(mem_start);
> -mem_reg_property[1] = cpu_to_be64(node_mem[i]);
> +if (mem_start >= ram_size) {
> +node_size = 0;
> +} else {
> +node_size = node_mem[i];
> +if (node_size > ram_size - mem_start) {
> +node_size = ram_size - mem_start;
> +}
> +}
> +mem_reg_property[1] = cpu_to_be64(node_size);
> associativity[3] = associativity[4] = cpu_to_be32(i);
> sprintf(mem_name, "memory@" TARGET_FMT_lx, mem_start);
> off = fdt_add_subnode(fdt, 0, mem_name);
> @@ -573,7 +585,7 @@ static int spapr_populate_memory(sPAPREnvironment *spapr,
> void *fdt)
>sizeof(mem_reg_property;
> _FDT((fdt_setprop(fdt, off, "ibm,associativity", associativity,
> sizeof(associativity;
> -mem_start += node_mem[i];
> +mem_start += node_size;
> }
>
> return 0;
Apart from the typo in the commit message, the patch looks good to me.
Reviewed-by: Thomas Huth
Re: [Qemu-devel] [PATCH] kvm: Query KVM for available memory slots
On Fri, 22 Nov 2013 12:12:44 -0700
Alex Williamson wrote:
> KVM reports the number of available memory slots (KVM_CAP_NR_MEMSLOTS)
> using the extension interface. Both x86 and s390 implement this, ARM
> and powerpc do not yet enable it. Convert the static slots array to
> be dynamically allocated, supporting more slots when available.
> Default to 32 when KVM_CAP_NR_MEMSLOTS is not implemented. The
> motivation for this change is to support more assigned devices, where
> memory mapped PCI MMIO BARs typically take one slot each.
>
> Signed-off-by: Alex Williamson
> ---
> kvm-all.c | 30 +-
> 1 file changed, 21 insertions(+), 9 deletions(-)
>
> diff --git a/kvm-all.c b/kvm-all.c
> index 4478969..63c4e9b 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -72,7 +72,8 @@ typedef struct kvm_dirty_log KVMDirtyLog;
>
> struct KVMState
> {
> -KVMSlot slots[32];
> +KVMSlot *slots;
> +int nr_slots;
> int fd;
> int vmfd;
> int coalesced_mmio;
> @@ -125,7 +126,7 @@ static KVMSlot *kvm_alloc_slot(KVMState *s)
> {
> int i;
>
> -for (i = 0; i < ARRAY_SIZE(s->slots); i++) {
> +for (i = 0; i < s->nr_slots; i++) {
> if (s->slots[i].memory_size == 0) {
> return &s->slots[i];
> }
> @@ -141,7 +142,7 @@ static KVMSlot *kvm_lookup_matching_slot(KVMState *s,
> {
> int i;
>
> -for (i = 0; i < ARRAY_SIZE(s->slots); i++) {
> +for (i = 0; i < s->nr_slots; i++) {
> KVMSlot *mem = &s->slots[i];
>
> if (start_addr == mem->start_addr &&
> @@ -163,7 +164,7 @@ static KVMSlot *kvm_lookup_overlapping_slot(KVMState *s,
> KVMSlot *found = NULL;
> int i;
>
> -for (i = 0; i < ARRAY_SIZE(s->slots); i++) {
> +for (i = 0; i < s->nr_slots; i++) {
> KVMSlot *mem = &s->slots[i];
>
> if (mem->memory_size == 0 ||
> @@ -185,7 +186,7 @@ int kvm_physical_memory_addr_from_host(KVMState *s, void
> *ram,
> {
> int i;
>
> -for (i = 0; i < ARRAY_SIZE(s->slots); i++) {
> +for (i = 0; i < s->nr_slots; i++) {
> KVMSlot *mem = &s->slots[i];
>
> if (ram >= mem->ram && ram < mem->ram + mem->memory_size) {
> @@ -357,7 +358,7 @@ static int kvm_set_migration_log(int enable)
>
> s->migration_log = enable;
>
> -for (i = 0; i < ARRAY_SIZE(s->slots); i++) {
> +for (i = 0; i < s->nr_slots; i++) {
> mem = &s->slots[i];
>
> if (!mem->memory_size) {
> @@ -1383,9 +1384,6 @@ int kvm_init(void)
> #ifdef KVM_CAP_SET_GUEST_DEBUG
> QTAILQ_INIT(&s->kvm_sw_breakpoints);
> #endif
> -for (i = 0; i < ARRAY_SIZE(s->slots); i++) {
> -s->slots[i].slot = i;
> -}
> s->vmfd = -1;
> s->fd = qemu_open("/dev/kvm", O_RDWR);
> if (s->fd == -1) {
> @@ -1409,6 +1407,19 @@ int kvm_init(void)
> goto err;
> }
>
> +s->nr_slots = kvm_check_extension(s, KVM_CAP_NR_MEMSLOTS);
> +
> +/* If unspecified, use the previous default value */
I'd remove the "previous" in the comment here, because if you look at
this code in a couple of years, nobody will remember what "previous" is
refering to here.
> +if (!s->nr_slots) {
> +s->nr_slots = 32;
> +}
> +
> +s->slots = g_malloc0(s->nr_slots * sizeof(KVMSlot));
> +
> +for (i = 0; i < s->nr_slots; i++) {
> +s->slots[i].slot = i;
> +}
> +
> /* check the vcpu limits */
> soft_vcpus_limit = kvm_recommended_vcpus(s);
> hard_vcpus_limit = kvm_max_vcpus(s);
> @@ -1527,6 +1538,7 @@ err:
> if (s->fd != -1) {
> close(s->fd);
> }
> +g_free(s->slots);
> g_free(s);
>
> return ret;
Reviewed-by: Thomas Huth
Re: [Qemu-devel] [PATCH 3/3] s390x/helper: Added format control bit to MMU translation
Hi Alex,
On Thu, 24 Apr 2014 18:36:18 +0200
Alexander Graf wrote:
>
> On 24.04.14 17:34, Jens Freimann wrote:
> > From: Thomas Huth
> >
> > With the EDAT-1 facility, the MMU translation can stop at the
> > segment table already, pointing to a 1 MB block.
...
> > diff --git a/target-s390x/helper.c b/target-s390x/helper.c
> > index aa628b8..89dc6e7 100644
> > --- a/target-s390x/helper.c
> > +++ b/target-s390x/helper.c
> > @@ -217,6 +217,10 @@ static int mmu_translate_asce(CPUS390XState *env,
> > target_ulong vaddr,
> > offs = (vaddr >> 17) & 0x3ff8;
> > break;
> > case _ASCE_TYPE_SEGMENT:
> > +if (env && (env->cregs[0] & CR0_EDAT) && (asce &
> > _SEGMENT_ENTRY_FC)) {
> > +*raddr = (asce & 0xfff0ULL) | (vaddr & 0xf);
> > +return 0;
> > +}
>
> This is missing the page flags.
D'oh, missed that :-(
> I also think we should rather align the
> code with the PTE handling somehow. This way it gets pretty confusing to
> follow. How about something like this (untested)?
I gave it a try ... works fine with two small modifications...
> diff --git a/target-s390x/helper.c b/target-s390x/helper.c
> index aa628b8..96c1c66 100644
> --- a/target-s390x/helper.c
> +++ b/target-s390x/helper.c
> @@ -170,6 +170,48 @@ static void trigger_page_fault(CPUS390XState *env,
> target_ulong vaddr,
> trigger_pgm_exception(env, type, ilen);
> }
>
> +static int mmu_translate_pte(CPUS390XState *env, target_ulong vaddr,
> + uint64_t asc, uint64_t asce,
> + target_ulong *raddr, int *flags, int rw)
> +{
> +if (asce & _PAGE_INVALID) {
> +DPRINTF("%s: PTE=0x%" PRIx64 " invalid\n", __func__, asce);
> +trigger_page_fault(env, vaddr, PGM_PAGE_TRANS, asc, rw);
> +return -1;
> +}
> +
> +if (asce & _PAGE_RO) {
> +*flags &= ~PAGE_WRITE;
> +}
> +
> +*raddr = asce & _ASCE_ORIGIN;
> +
> +PTE_DPRINTF("%s: PTE=0x%" PRIx64 "\n", __func__, asce);
> +
> +return 0;
> +}
> +
> +static int mmu_translate_segpte(CPUS390XState *env, target_ulong vaddr,
> +uint64_t asc, uint64_t asce,
> + target_ulong *raddr, int *flags, int rw)
> +{
> +if (asce & _SEGMENT_ENTRY_INV) {
> +DPRINTF("%s: SEG=0x%" PRIx64 " invalid\n", __func__, asce);
> +trigger_page_fault(env, vaddr, PGM_SEGMENT_TRANS, asc, rw);
> +return -1;
> +}
> +
> +if (asce & _PAGE_RO) { /* XXX is this correct? */
You can remove the XXX comment, the protection bit is the same for
both, page table entries and segment table entries.
> +*flags &= ~PAGE_WRITE;
> +}
> +
> +*raddr = (asce & 0xfff0ULL) | (vaddr & 0xf);
> +
> +PTE_DPRINTF("%s: SEG=0x%" PRIx64 "\n", __func__, asce);
> +
> +return 0;
> +}
> +
> static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr,
> uint64_t asc, uint64_t asce, int level,
> target_ulong *raddr, int *flags, int rw)
> @@ -229,28 +271,19 @@ static int mmu_translate_asce(CPUS390XState *env,
> target_ulong vaddr,
> PTE_DPRINTF("%s: 0x%" PRIx64 " + 0x%" PRIx64 " => 0x%016" PRIx64 "\n",
> __func__, origin, offs, new_asce);
>
> -if (level != _ASCE_TYPE_SEGMENT) {
> +} if (level == _ASCE_TYPE_SEGMENT) {
I had to remove the "}" at the beginning of above line.
> +/* 4KB page */
> +return mmu_translate_pte(env, vaddr, asc, new_asce, raddr,
> flags, rw);
> +} else if (((level - 4) == _ASCE_TYPE_SEGMENT) &&
> + (env->cregs[0] & CR0_EDAT) &&
> +(asce & _SEGMENT_ENTRY_FC)) {
That's got to be "(new_asce & _SEGMENT_ENTRY_FC)" instead.
> +/* 1MB page */
> +return mmu_translate_segpte(env, vaddr, asc, new_asce, raddr,
> flags, rw);
> +} else {
> /* yet another region */
> return mmu_translate_asce(env, vaddr, asc, new_asce, level -
> 4, raddr,
> flags, rw);
> }
> -
> -/* PTE */
> -if (new_asce & _PAGE_INVALID) {
> -DPRINTF("%s: PTE=0x%" PRIx64 " invalid\n", __func__, new_asce);
> -trigger_page_fault(env, vaddr, PGM_PAGE_TRANS, asc, rw);
> -return -1;
> -}
> -
> -if (new_asce & _PAGE_RO) {
> -*flags &= ~PAGE_WRITE;
> -}
> -
> -*raddr = new_asce & _ASCE_ORIGIN;
> -
> -PTE_DPRINTF("%s: PTE=0x%" PRIx64 "\n", __func__, new_asce);
> -
> -return 0;
> }
>
> static int mmu_translate_asc(CPUS390XState *env, target_ulong vaddr,
I'm fine with these changes, too. So how shall we continue? Could you
assemble a complete patch or shall I prepare one?
Thomas
Re: [Qemu-devel] [PATCH 3/3] s390x/helper: Added format control bit to MMU translation
On Fri, 25 Apr 2014 14:36:11 +0200
Alexander Graf wrote:
>
> On 25.04.14 14:15, Thomas Huth wrote:
> >
> > On Thu, 24 Apr 2014 18:36:18 +0200
> > Alexander Graf wrote:
[...]
> >> I also think we should rather align the
> >> code with the PTE handling somehow. This way it gets pretty confusing to
> >> follow. How about something like this (untested)?
> > I gave it a try ... works fine with two small modifications...
> >
> >> diff --git a/target-s390x/helper.c b/target-s390x/helper.c
> >> index aa628b8..96c1c66 100644
> >> --- a/target-s390x/helper.c
> >> +++ b/target-s390x/helper.c
> >> @@ -170,6 +170,48 @@ static void trigger_page_fault(CPUS390XState *env,
> >> target_ulong vaddr,
> >>trigger_pgm_exception(env, type, ilen);
> >>}
> >>
> >> +static int mmu_translate_pte(CPUS390XState *env, target_ulong vaddr,
> >> + uint64_t asc, uint64_t asce,
> >> + target_ulong *raddr, int *flags, int rw)
> >> +{
> >> +if (asce & _PAGE_INVALID) {
> >> +DPRINTF("%s: PTE=0x%" PRIx64 " invalid\n", __func__, asce);
> >> +trigger_page_fault(env, vaddr, PGM_PAGE_TRANS, asc, rw);
> >> +return -1;
> >> +}
> >> +
> >> +if (asce & _PAGE_RO) {
> >> +*flags &= ~PAGE_WRITE;
> >> +}
> >> +
> >> +*raddr = asce & _ASCE_ORIGIN;
> >> +
> >> +PTE_DPRINTF("%s: PTE=0x%" PRIx64 "\n", __func__, asce);
> >> +
> >> +return 0;
> >> +}
> >> +
> >> +static int mmu_translate_segpte(CPUS390XState *env, target_ulong vaddr,
> >> +uint64_t asc, uint64_t asce,
> >> + target_ulong *raddr, int *flags, int rw)
> >> +{
> >> +if (asce & _SEGMENT_ENTRY_INV) {
> >> +DPRINTF("%s: SEG=0x%" PRIx64 " invalid\n", __func__, asce);
> >> +trigger_page_fault(env, vaddr, PGM_SEGMENT_TRANS, asc, rw);
> >> +return -1;
> >> +}
> >> +
> >> +if (asce & _PAGE_RO) { /* XXX is this correct? */
> > You can remove the XXX comment, the protection bit is the same for
> > both, page table entries and segment table entries.
> >
> >> +*flags &= ~PAGE_WRITE;
> >> +}
> >> +
> >> +*raddr = (asce & 0xfff0ULL) | (vaddr & 0xf);
> >> +
> >> +PTE_DPRINTF("%s: SEG=0x%" PRIx64 "\n", __func__, asce);
> >> +
> >> +return 0;
> >> +}
> >> +
> >>static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr,
> >> uint64_t asc, uint64_t asce, int level,
> >> target_ulong *raddr, int *flags, int rw)
> >> @@ -229,28 +271,19 @@ static int mmu_translate_asce(CPUS390XState *env,
> >> target_ulong vaddr,
> >>PTE_DPRINTF("%s: 0x%" PRIx64 " + 0x%" PRIx64 " => 0x%016" PRIx64
> >> "\n",
> >>__func__, origin, offs, new_asce);
> >>
> >> -if (level != _ASCE_TYPE_SEGMENT) {
> >> +} if (level == _ASCE_TYPE_SEGMENT) {
> > I had to remove the "}" at the beginning of above line.
> >
> >> +/* 4KB page */
> >> +return mmu_translate_pte(env, vaddr, asc, new_asce, raddr,
> >> flags, rw);
> >> +} else if (((level - 4) == _ASCE_TYPE_SEGMENT) &&
> >> + (env->cregs[0] & CR0_EDAT) &&
> >> +(asce & _SEGMENT_ENTRY_FC)) {
> > That's got to be "(new_asce & _SEGMENT_ENTRY_FC)" instead.
> >
> >> +/* 1MB page */
> >> +return mmu_translate_segpte(env, vaddr, asc, new_asce, raddr,
> >> flags, rw);
> >> +} else {
> >>/* yet another region */
> >>return mmu_translate_asce(env, vaddr, asc, new_asce, level -
> >> 4, raddr,
> >> flags, rw);
> >>}
> >> -
> >> -/* PTE */
> >> -if (new_asce & _PAGE_INVALID) {
> >> -DPRINTF("%s: PTE=0x%" PRIx64 " invalid\n", __func__, new_asce);
> >> -trigger_page_fault(env, vaddr, PGM_PAGE_TRANS, asc, rw);
> >
[Qemu-devel] [PATCH] s390x/helper: Added format control bit to MMU translation
With the EDAT-1 facility, the MMU translation can stop at the
segment table already, pointing to a 1 MB block. And while we're
at it, move the page table entry handling to a separate function,
too, as suggested by Alexander Graf.
Signed-off-by: Thomas Huth
---
target-s390x/cpu.h|4 +++
target-s390x/helper.c | 70
2 files changed, 56 insertions(+), 18 deletions(-)
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index f332d41..686d458 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -267,6 +267,9 @@ typedef struct CPUS390XState {
#define FLAG_MASK_64(PSW_MASK_64 >> 32)
#define FLAG_MASK_320x1000
+/* Control register 0 bits */
+#define CR0_EDAT0x0080ULL
+
static inline int cpu_mmu_index (CPUS390XState *env)
{
if (env->psw.mask & PSW_MASK_PSTATE) {
@@ -924,6 +927,7 @@ struct sysib_322 {
#define _REGION_ENTRY_LENGTH0x03 /* region third length
*/
#define _SEGMENT_ENTRY_ORIGIN ~0x7ffULL /* segment table origin
*/
+#define _SEGMENT_ENTRY_FC 0x400 /* format control
*/
#define _SEGMENT_ENTRY_RO 0x200 /* page protection bit
*/
#define _SEGMENT_ENTRY_INV 0x20 /* invalid segment table entry
*/
diff --git a/target-s390x/helper.c b/target-s390x/helper.c
index ddf268e..53e6615 100644
--- a/target-s390x/helper.c
+++ b/target-s390x/helper.c
@@ -184,6 +184,50 @@ static target_ulong mmu_real2abs(CPUS390XState *env,
target_ulong raddr)
return raddr;
}
+/* Decode page table entry (normal 4KB page) */
+static int mmu_translate_pte(CPUS390XState *env, target_ulong vaddr,
+ uint64_t asc, uint64_t asce,
+ target_ulong *raddr, int *flags, int rw)
+{
+if (asce & _PAGE_INVALID) {
+DPRINTF("%s: PTE=0x%" PRIx64 " invalid\n", __func__, asce);
+trigger_page_fault(env, vaddr, PGM_PAGE_TRANS, asc, rw);
+return -1;
+}
+
+if (asce & _PAGE_RO) {
+*flags &= ~PAGE_WRITE;
+}
+
+*raddr = asce & _ASCE_ORIGIN;
+
+PTE_DPRINTF("%s: PTE=0x%" PRIx64 "\n", __func__, asce);
+
+return 0;
+}
+
+/* Decode EDAT1 segment frame absolute address (1MB page) */
+static int mmu_translate_sfaa(CPUS390XState *env, target_ulong vaddr,
+ uint64_t asc, uint64_t asce, target_ulong *raddr,
+ int *flags, int rw)
+{
+if (asce & _SEGMENT_ENTRY_INV) {
+DPRINTF("%s: SEG=0x%" PRIx64 " invalid\n", __func__, asce);
+trigger_page_fault(env, vaddr, PGM_SEGMENT_TRANS, asc, rw);
+return -1;
+}
+
+if (asce & _SEGMENT_ENTRY_RO) {
+*flags &= ~PAGE_WRITE;
+}
+
+*raddr = (asce & 0xfff0ULL) | (vaddr & 0xf);
+
+PTE_DPRINTF("%s: SEG=0x%" PRIx64 "\n", __func__, asce);
+
+return 0;
+}
+
static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr,
uint64_t asc, uint64_t asce, int level,
target_ulong *raddr, int *flags, int rw)
@@ -243,28 +287,18 @@ static int mmu_translate_asce(CPUS390XState *env,
target_ulong vaddr,
PTE_DPRINTF("%s: 0x%" PRIx64 " + 0x%" PRIx64 " => 0x%016" PRIx64 "\n",
__func__, origin, offs, new_asce);
-if (level != _ASCE_TYPE_SEGMENT) {
+if (level == _ASCE_TYPE_SEGMENT) {
+/* 4KB page */
+return mmu_translate_pte(env, vaddr, asc, new_asce, raddr, flags, rw);
+} else if (level - 4 == _ASCE_TYPE_SEGMENT &&
+ (new_asce & _SEGMENT_ENTRY_FC) && (env->cregs[0] & CR0_EDAT)) {
+/* 1MB page */
+return mmu_translate_sfaa(env, vaddr, asc, new_asce, raddr, flags, rw);
+} else {
/* yet another region */
return mmu_translate_asce(env, vaddr, asc, new_asce, level - 4, raddr,
flags, rw);
}
-
-/* PTE */
-if (new_asce & _PAGE_INVALID) {
-DPRINTF("%s: PTE=0x%" PRIx64 " invalid\n", __func__, new_asce);
-trigger_page_fault(env, vaddr, PGM_PAGE_TRANS, asc, rw);
-return -1;
-}
-
-if (new_asce & _PAGE_RO) {
-*flags &= ~PAGE_WRITE;
-}
-
-*raddr = new_asce & _ASCE_ORIGIN;
-
-PTE_DPRINTF("%s: PTE=0x%" PRIx64 "\n", __func__, new_asce);
-
-return 0;
}
static int mmu_translate_asc(CPUS390XState *env, target_ulong vaddr,
--
1.7.1
Re: [Qemu-devel] [PATCH] spapr: make sure RMA is in first mode of first memory node
On Mon, 4 Nov 2013 12:28:12 +0100
Alexander Graf wrote:
>
> On 04.11.2013, at 11:55, Benjamin Herrenschmidt
> wrote:
>
> > On Mon, 2013-11-04 at 11:44 +0100, Alexander Graf wrote:
> >> On 01.11.2013, at 11:21, Alexey Kardashevskiy wrote:
> >>
> >>> SLOF gets really confused if RTAS/device-tree and everything else
> >>> what SLOF can use is not in the very first block of the very first
> >>> memory node.
> >>>
> >>> This makes sure that the RMA area is where SLOF expects it to be.
> >>>
> >>> Cc: Benjamin Herrenschmidt
> >>> Cc: Nikunj A Dadhania
> >>> Signed-off-by: Alexey Kardashevskiy
> >>> ---
> >>> hw/ppc/spapr.c | 8 +++-
> >>> 1 file changed, 7 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> >>> index 09dc635..09a5d94 100644
> >>> --- a/hw/ppc/spapr.c
> >>> +++ b/hw/ppc/spapr.c
> >>> @@ -1113,7 +1113,7 @@ static void ppc_spapr_init(QEMUMachineInitArgs
> >>> *args)
> >>>int i;
> >>>MemoryRegion *sysmem = get_system_memory();
> >>>MemoryRegion *ram = g_new(MemoryRegion, 1);
> >>> -hwaddr rma_alloc_size;
> >>> +hwaddr rma_alloc_size, node0_size;
> >>>uint32_t initrd_base = 0;
> >>>long kernel_size = 0, initrd_size = 0;
> >>>long load_limit, rtas_limit, fw_size;
> >>> @@ -1154,6 +1154,12 @@ static void ppc_spapr_init(QEMUMachineInitArgs
> >>> *args)
> >>>spapr->rma_size = MIN(spapr->rma_size, 0x1000);
> >>>}
> >>>}
> >>> +/*
> >>> + * SLOF gets confused if RMA resides not in the first block
> >>> + * of the first memory node so let's fix it.
> >>> + */
> >>> +node0_size = (nb_numa_nodes > 1) ? node_mem[0] : ram_size;
> >>> +spapr->rma_size = MIN(spapr->rma_size, node0_size);
> >> So if I create a NUMA node of 4MB that will be my RMA? That sounds pretty
> >> broken, especially on 970.
> >>
> >> Why does SLOF have any issues with NUMA memory nodes? It can just ignore
> >> them, no?
> >
> > Because the only way SLOF knows about the RMA is by using the first
> > "reg" entry of the first memory node and that's *all* SLOF knows about.
> >
> > If we start putting things like the DT, SLOF itself, etc... outside of
> > that region, it will crash.
Ok, the question is whether this is a bug in SLOF and should be fixed
there or whether the RMA should really be limited to the RAM of the
first node only.
Looking at the function spapr_populate_memory(), it seems there is
already similar code there, so I assume the RMA should really be
limited to that size:
/* memory node(s) */
node0_size = (nb_numa_nodes > 1) ? node_mem[0] : ram_size;
if (spapr->rma_size > node0_size) {
spapr->rma_size = node0_size;
}
Maybe this piece of code could just be done earlier instead, before
setting up the fdt_addr and rtas_addr variables, instead of adding the
similar piece of code of this patch?
> > So we "constrain" things to the rma that way.
> >
> > Creating 4M nodes makes no sense anyway
>
> So why don't we just use the "limit VRMA to 256MB" code always and error out
> of node0 is smaller? I don't think SLOF can run with less than 256MB anyway.
It's 128 MB nowadays ... there is even a define called MIN_RMA_SLOF for
this in the code already.
Thomas
Re: [Qemu-devel] [PATCH 7/8] spapr: remove @next_irq
On Fri, 14 Mar 2014 15:18:08 +1100
Alexey Kardashevskiy wrote:
> This removes @next_irq from sPAPREnvironment which was used in old
> IRQ allocator as XICS is now responsible for IRQs and keep track of
> allocated IRQs.
>
> Signed-off-by: Alexey Kardashevskiy
> ---
> hw/ppc/spapr.c | 3 ---
> include/hw/ppc/spapr.h | 1 -
> 2 files changed, 4 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 29ca2e0..1f7162c 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -732,8 +732,6 @@ static const VMStateDescription vmstate_spapr = {
> .minimum_version_id = 1,
> .minimum_version_id_old = 1,
> .fields = (VMStateField []) {
> -VMSTATE_UINT32(next_irq, sPAPREnvironment),
> -
You're changing the layout of a state description here... maybe it
would be a good idea to increase the version_id (and
minimum_version_id) in that case?
Thomas
Re: [Qemu-devel] How to understand the coroutine context?
On Tue, 18 Mar 2014 09:34:56 +0530 Kashyap Chamarthy wrote: > On Tue, Mar 18, 2014 at 07:56:16AM +0800, Le Tan wrote: > > Hi, I am diving into the source code of qemu. I see the word > > "coroutine" appears in so many places. I can't figure out what it > > means. So, please, can anyone help me, telling me the mechanism or > > semantic of "coroutine"? Thanks! > > This might be of help -- > http://blog.vmsplice.net/2014/01/coroutines-in-qemu-basics.html That's a very nice introduction ... Stefan, maybe you could also add that to the docs directory of the QEMU repository? There does not seem to be a description of the QEMU coroutines there yet... Regards, Thomas
Re: [Qemu-devel] How to understand the coroutine context?
On Wed, 19 Mar 2014 11:05:02 +0100 Stefan Hajnoczi wrote: > On Tue, Mar 18, 2014 at 11:28:47PM +0100, Thomas Huth wrote: > > On Tue, 18 Mar 2014 09:34:56 +0530 > > Kashyap Chamarthy wrote: > > > > > On Tue, Mar 18, 2014 at 07:56:16AM +0800, Le Tan wrote: > > > > Hi, I am diving into the source code of qemu. I see the word > > > > "coroutine" appears in so many places. I can't figure out what it > > > > means. So, please, can anyone help me, telling me the mechanism or > > > > semantic of "coroutine"? Thanks! > > > > > > This might be of help -- > > > http://blog.vmsplice.net/2014/01/coroutines-in-qemu-basics.html > > > > That's a very nice introduction ... Stefan, maybe you could also add > > that to the docs directory of the QEMU repository? There does not seem > > to be a description of the QEMU coroutines there yet... > > Have you seen the documentation in include/block/coroutine.h? Yes, but that does not have the nice introduction that you've given in your blog - that's why I though it might be useful to have that information in a file in the docs directory, too. Well, it was just an idea ... if you think the API description in coroutine.h is sufficient, I am certainly fine with that. Regards, Thomas
[Qemu-devel] [PATCH] KVM: s390: Add MEMOP ioctls for reading/writing guest memory
On s390, we've got to make sure to hold the IPTE lock while accessing
logical memory. So let's add an ioctl for reading and writing logical
memory to provide this feature for userspace, too.
Signed-off-by: Thomas Huth
---
Documentation/virtual/kvm/api.txt | 45 +++
arch/s390/kvm/gaccess.c | 22 +++
arch/s390/kvm/gaccess.h |2 +
arch/s390/kvm/kvm-s390.c | 70 +
include/uapi/linux/kvm.h | 20 ++
5 files changed, 159 insertions(+), 0 deletions(-)
diff --git a/Documentation/virtual/kvm/api.txt
b/Documentation/virtual/kvm/api.txt
index b112efc..8aa99f3 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -2716,6 +2716,51 @@ The fields in each entry are defined as follows:
eax, ebx, ecx, edx: the values returned by the cpuid instruction for
this function/index combination
+4.89 KVM_S390_MEM_OP
+
+Capability: KVM_CAP_S390_MEM_OP
+Architectures: s390
+Type: vcpu ioctl
+Parameters: struct kvm_s390_mem_op (in)
+Returns: = 0 on success,
+ < 0 on generic error (e.g. -EFAULT or -ENOMEM),
+ > 0 if an exception occurred while walking the page tables
+
+Read or write data from/to the logical (virtual) memory of a VPCU.
+
+Parameters are specified via the following structure:
+
+struct kvm_s390_mem_op {
+ __u64 gaddr;/* the guest address */
+ __u64 flags;/* arch specific flags */
+ __u32 size; /* amount of bytes */
+ __u32 op; /* type of operation */
+ __u64 buf; /* buffer in userspace */
+ __u8 reserved[32]; /* should be set to 0 */
+};
+
+The type of operation is specified in the "op" field. It is either
+KVM_S390_MEMOP_LOGICAL_READ for reading from logical memory space or
+KVM_S390_MEMOP_LOGICAL_WRITE for writing to logical memory space. The
+KVM_S390_MEMOP_F_CHECK_ONLY flag can be set in the "flags" field to check
+whether the corresponding memory access would create an access exception
+(without touching the data in the memory at the destination). In case an
+access exception occurred while walking the MMU tables of the guest, the
+ioctl returns a positive error number to indicate the type of exception.
+This exception is also raised directly at the corresponding VCPU if the
+flag KVM_S390_MEMOP_F_INJECT_EXCEPTION is set in the "flags" field.
+
+The start address of the memory region has to be specified in the "gaddr"
+field, and the length of the region in the "size" field. "buf" is the buffer
+supplied by the userspace application where the read data should be written
+to for KVM_S390_MEMOP_LOGICAL_READ, or where the data that should be written
+is stored for a KVM_S390_MEMOP_LOGICAL_WRITE. "buf" is unused and can be NULL
+when KVM_S390_MEMOP_F_CHECK_ONLY is specified.
+
+The "reserved" field is meant for future extensions. It is not used by
+KVM with the currently defined set of flags.
+
+
5. The kvm_run structure
diff --git a/arch/s390/kvm/gaccess.c b/arch/s390/kvm/gaccess.c
index 267523c..70fd956 100644
--- a/arch/s390/kvm/gaccess.c
+++ b/arch/s390/kvm/gaccess.c
@@ -697,6 +697,28 @@ int guest_translate_address(struct kvm_vcpu *vcpu,
unsigned long gva,
}
/**
+ * check_gva_range - test a range of guest virtual addresses for accessibility
+ */
+int check_gva_range(struct kvm_vcpu *vcpu, unsigned long gva,
+ unsigned long length, int is_write)
+{
+ unsigned long gpa;
+ unsigned long currlen;
+ int rc = 0;
+
+ ipte_lock(vcpu);
+ while (length > 0 && !rc) {
+ currlen = min(length, PAGE_SIZE - (gva % PAGE_SIZE));
+ rc = guest_translate_address(vcpu, gva, &gpa, is_write);
+ gva += currlen;
+ length -= currlen;
+ }
+ ipte_unlock(vcpu);
+
+ return rc;
+}
+
+/**
* kvm_s390_check_low_addr_protection - check for low-address protection
* @ga: Guest address
*
diff --git a/arch/s390/kvm/gaccess.h b/arch/s390/kvm/gaccess.h
index 0149cf1..268beb7 100644
--- a/arch/s390/kvm/gaccess.h
+++ b/arch/s390/kvm/gaccess.h
@@ -157,6 +157,8 @@ int read_guest_lc(struct kvm_vcpu *vcpu, unsigned long gra,
void *data,
int guest_translate_address(struct kvm_vcpu *vcpu, unsigned long gva,
unsigned long *gpa, int write);
+int check_gva_range(struct kvm_vcpu *vcpu, unsigned long gva,
+ unsigned long length, int is_write);
int access_guest(struct kvm_vcpu *vcpu, unsigned long ga, void *data,
unsigned long len, int write);
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 0c36239..fe9eb5a 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -25,6 +25,7 @@
#include
#include
#i
[Qemu-devel] [PATCH 1/2] Update Linux headers with KVM_GUEST_MEM_OP ioctl
Synchronized the kvm.h Linux header for the new ioctl.
Signed-off-by: Thomas Huth
---
linux-headers/linux/kvm.h | 21 +
1 files changed, 21 insertions(+), 0 deletions(-)
diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index 12045a1..a5f2c8e 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -365,6 +365,23 @@ struct kvm_translation {
__u8 pad[5];
};
+/* for KVM_S390_MEM_OP */
+struct kvm_s390_mem_op {
+ /* in */
+ __u64 gaddr;/* the guest address */
+ __u64 flags;/* arch specific flags */
+ __u32 size; /* amount of bytes */
+ __u32 op; /* type of operation */
+ __u64 buf; /* buffer in userspace */
+ __u8 reserved[32]; /* should be set to 0 */
+};
+/* types for kvm_s390_mem_op->op */
+#define KVM_S390_MEMOP_LOGICAL_READ0
+#define KVM_S390_MEMOP_LOGICAL_WRITE 1
+/* flags for kvm_s390_mem_op->flags */
+#define KVM_S390_MEMOP_F_CHECK_ONLY(1ULL << 0)
+#define KVM_S390_MEMOP_F_INJECT_EXCEPTION (1ULL << 1)
+
/* for KVM_INTERRUPT */
struct kvm_interrupt {
/* in */
@@ -761,6 +778,8 @@ struct kvm_ppc_smmu_info {
#define KVM_CAP_PPC_FIXUP_HCALL 103
#define KVM_CAP_PPC_ENABLE_HCALL 104
#define KVM_CAP_CHECK_EXTENSION_VM 105
+#define KVM_CAP_S390_USER_SIGP 106
+#define KVM_CAP_S390_MEM_OP 107
#ifdef KVM_CAP_IRQ_ROUTING
@@ -1137,6 +1156,8 @@ struct kvm_s390_ucas_mapping {
#define KVM_ARM_VCPU_INIT_IOW(KVMIO, 0xae, struct kvm_vcpu_init)
#define KVM_ARM_PREFERRED_TARGET _IOR(KVMIO, 0xaf, struct kvm_vcpu_init)
#define KVM_GET_REG_LIST _IOWR(KVMIO, 0xb0, struct kvm_reg_list)
+/* Available with KVM_CAP_S390_MEM_OP */
+#define KVM_S390_MEM_OP _IOW(KVMIO, 0xb1, struct
kvm_s390_mem_op)
#define KVM_DEV_ASSIGN_ENABLE_IOMMU(1 << 0)
#define KVM_DEV_ASSIGN_PCI_2_3 (1 << 1)
--
1.7.1
[Qemu-devel] [PATCH 0/2] s390x/kvm: ioctl for reading and writing from/to guest memory
These two patches enable QEMU to make use of the new KVM_GUEST_MEM_OP ioctl on s390x (see my kernel patch with the title "KVM: s390: Add MEMOP ioctls for reading/writing guest memory" that I've posted on the KVM mailing list). The first patch in this series updates the Linux headers with the new ioctl definitions. This should be replaced with a proper synchronized header update against kvm/next as soon as the kernel part has been accepted. The second patch then uses the ioctl when QEMU has to access the logical memory of a guest. This patch goes on top of my "s390x: rework guest memory access" patch series that has been sent out to the qemu-devel list by Jens Freimann on last thursday. Thomas Huth (2): Update Linux headers with KVM_GUEST_MEM_OP ioctl s390x/mmu: Use ioctl for reading and writing from/to guest memory linux-headers/linux/kvm.h | 21 + target-s390x/cpu.h|7 +++ target-s390x/kvm.c| 35 +++ target-s390x/mmu_helper.c |9 + 4 files changed, 72 insertions(+), 0 deletions(-)
[Qemu-devel] [PATCH 2/2] s390x/mmu: Use ioctl for reading and writing from/to guest memory
Add code to make use of the new ioctl for reading from / writing to
virtual guest memory. By using the ioctl, the memory accesses are now
protected with the so-called ipte-lock in the kernel.
Signed-off-by: Thomas Huth
---
target-s390x/cpu.h|7 +++
target-s390x/kvm.c| 35 +++
target-s390x/mmu_helper.c |9 +
3 files changed, 51 insertions(+), 0 deletions(-)
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index b6b4632..cffdd1b 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -394,6 +394,8 @@ void kvm_s390_service_interrupt(uint32_t parm);
void kvm_s390_vcpu_interrupt(S390CPU *cpu, struct kvm_s390_irq *irq);
void kvm_s390_floating_interrupt(struct kvm_s390_irq *irq);
int kvm_s390_inject_flic(struct kvm_s390_irq *irq);
+int kvm_s390_mem_op(S390CPU *cpu, vaddr addr, void *hostbuf, int len,
+bool is_write);
void kvm_s390_access_exception(S390CPU *cpu, uint16_t code, uint64_t te_code);
#else
static inline void kvm_s390_virtio_irq(int config_change, uint64_t token)
@@ -402,6 +404,11 @@ static inline void kvm_s390_virtio_irq(int config_change,
uint64_t token)
static inline void kvm_s390_service_interrupt(uint32_t parm)
{
}
+static inline int kvm_s390_mem_op(S390CPU *cpu, vaddr addr, void *hostbuf,
+ int len, bool is_write)
+{
+return -ENOSYS;
+}
static inline void kvm_s390_access_exception(S390CPU *cpu, uint16_t code,
uint64_t te_code)
{
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 195493c..57a3aa1 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -117,6 +117,7 @@ const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
static int cap_sync_regs;
static int cap_async_pf;
+static int cap_mem_op;
static void *legacy_s390_alloc(size_t size, uint64_t *align);
@@ -176,6 +177,7 @@ int kvm_arch_init(KVMState *s)
{
cap_sync_regs = kvm_check_extension(s, KVM_CAP_SYNC_REGS);
cap_async_pf = kvm_check_extension(s, KVM_CAP_ASYNC_PF);
+cap_mem_op = kvm_check_extension(s, KVM_CAP_S390_MEM_OP);
if (kvm_check_extension(s, KVM_CAP_VM_ATTRIBUTES)) {
kvm_s390_enable_cmma(s);
@@ -436,6 +438,39 @@ int kvm_arch_get_registers(CPUState *cs)
return 0;
}
+/**
+ * kvm_s390_mem_op:
+ * @addr: the logical start address in guest memory
+ * @hostbuf: buffer in host memory. NULL = do only checks w/o copying
+ * @len: length that should be transfered
+ * @is_write: true = write, false = read
+ * Returns:0 on success, non-zero if an exception or error occured
+ *
+ * Use KVM ioctl to read/write from/to guest memory. An access exception
+ * is injected into the vCPU in case of translation errors.
+ */
+int kvm_s390_mem_op(S390CPU *cpu, vaddr addr, void *hostbuf, int len,
+bool is_write)
+{
+struct kvm_s390_mem_op mem_op = {
+.gaddr = addr,
+.flags = KVM_S390_MEMOP_F_INJECT_EXCEPTION,
+.size = len,
+.op = is_write ? KVM_S390_MEMOP_LOGICAL_WRITE
+ : KVM_S390_MEMOP_LOGICAL_READ,
+.buf = (uint64_t)hostbuf,
+};
+
+if (!cap_mem_op) {
+return -ENOSYS;
+}
+if (!hostbuf) {
+mem_op.flags |= KVM_S390_MEMOP_F_CHECK_ONLY;
+}
+
+return kvm_vcpu_ioctl(CPU(cpu), KVM_S390_MEM_OP, &mem_op);
+}
+
/*
* Legacy layout for s390:
* Older S390 KVM requires the topmost vma of the RAM to be
diff --git a/target-s390x/mmu_helper.c b/target-s390x/mmu_helper.c
index b061c85..22f965a 100644
--- a/target-s390x/mmu_helper.c
+++ b/target-s390x/mmu_helper.c
@@ -450,6 +450,15 @@ int s390_cpu_virt_mem_rw(S390CPU *cpu, vaddr laddr, void
*hostbuf,
target_ulong *pages;
int ret;
+if (kvm_enabled()) {
+ret = kvm_s390_mem_op(cpu, laddr, hostbuf, len, is_write);
+if (ret >= 0) {
+return ret;
+} else if (ret != -ENOSYS) {
+error_printf("kvm_s390_mem_op() failed: %s\n", strerror(-ret));
+}
+}
+
nr_pages = (((laddr & ~TARGET_PAGE_MASK) + len - 1) >> TARGET_PAGE_BITS)
+ 1;
pages = g_malloc(nr_pages * sizeof(*pages));
--
1.7.1
Re: [Qemu-devel] [PATCH v2 01/17] scripts/update-linux-headers.sh: pull virtio hdrs
On Sun, 15 Feb 2015 12:38:34 +0100 "Michael S. Tsirkin" wrote: > It doesn't make sense to copy values manually: > the only issue with getting headers from linux > seems to be dealing with linux/types, we > can easily fix that automatically while importing. > > Signed-off-by: Michael S. Tsirkin > --- > scripts/update-linux-headers.sh | 33 + > 1 file changed, 33 insertions(+) > > diff --git a/scripts/update-linux-headers.sh b/scripts/update-linux-headers.sh > index c8e026d..738e590 100755 > --- a/scripts/update-linux-headers.sh > +++ b/scripts/update-linux-headers.sh > @@ -76,4 +76,37 @@ else > cp "$linux/COPYING" "$output/linux-headers" > fi > > + > +rm -rf "$output/include/standard-headers/sys" > +mkdir -p "$output/include/standard-headers/sys" > +for f in $tmpdir/include/linux/virtio*h; do > +if > +grep '#include' "$f" | grep -v -e 'linux/virtio' \ > + -e 'linux/types' \ > + -e 'linux/if_ether' \ > + > /dev/null > +then > +echo "Unexpected #include in input file $f". > +exit 2 > +fi > + > +header=$(expr "$f" : '.*/\(.*\)'); I think you could also use "basename $f" instead - that would be easier to read. > +sed -e 's/__u\([0-9][0-9]*\)/uint\1_t/g' \ > +-e 's/__le\([0-9][0-9]*\)/uint\1_t/g' \ > +-e 's/__be\([0-9][0-9]*\)/uint\1_t/g' \ > +-e 's/]*\)>/"standard-headers\/sys\/\1"/' \ > +-e 's/__bitwise__//' \ > +-e 's/__attribute__((packed))/QEMU_PACKED/' \ > +"$tmpdir/include/linux/$header" > \ > +"$output/include/standard-headers/sys/$header"; > +done > + > +cat <$output/include/standard-headers/sys/types.h > +#include This is just about getting the uintxx_t types, right? If so, I think it is also sufficient to just include instead. > +#include "qemu/compiler.h" > +EOF > +cat <$output/include/standard-headers/sys/if_ether.h > +#define ETH_ALEN6 > +EOF > + > rm -rf "$tmpdir" Reviewed-by: Thomas Huth
Re: [Qemu-devel] [PATCH v2 15/17] scripts: add arch specific standard-headers
On Sun, 15 Feb 2015 12:39:25 +0100
"Michael S. Tsirkin" wrote:
> Move virtio header copying logic to a function,
> use that to copy arch specific virtio headers.
>
> Signed-off-by: Michael S. Tsirkin
> ---
> scripts/update-linux-headers.sh | 56
> -
> 1 file changed, 33 insertions(+), 23 deletions(-)
>
> diff --git a/scripts/update-linux-headers.sh b/scripts/update-linux-headers.sh
> index 9654553..1be4d83 100755
> --- a/scripts/update-linux-headers.sh
> +++ b/scripts/update-linux-headers.sh
> @@ -28,6 +28,36 @@ if [ -z "$output" ]; then
> output="$PWD"
> fi
>
> +cp_virtio() {
> +from=$1
> +to=$2
> +virtio=$(find "$from" -name '*virtio*h')
> +if [ "$virtio" ]; then
> +rm -rf "$to"
> +mkdir -p "$to"
> +for f in $virtio; do
> +if
> +grep '#include' "$f" | grep -v -e 'linux/virtio' \
> + -e 'linux/types' \
> + -e 'linux/if_ether' \
> + > /dev/null
> +then
> +echo "Unexpected #include in input file $f".
> +exit 2
> +fi
> +
> +header=$(expr "$f" : '.*/\(.*\)');
> +sed -e 's/__u\([0-9][0-9]*\)/uint\1_t/g' \
> +-e 's/__le\([0-9][0-9]*\)/uint\1_t/g' \
> +-e 's/__be\([0-9][0-9]*\)/uint\1_t/g' \
> +-e 's/]*\)>/"standard-headers\/sys\/\1"/' \
> +-e 's/__bitwise__//' \
> +-e 's/__attribute__((packed))/QEMU_PACKED/' \
> +"$f" > "$to/$header";
> +done
> +fi
> +}
Could you maybe introduce this function in patch 1 already? That would
avoid a bit of code churn here.
Thomas
Re: [Qemu-devel] [PATCH v2 02/17] include: import virtio headers from linux 4.0
On Sun, 15 Feb 2015 12:38:37 +0100 "Michael S. Tsirkin" wrote: > Add files imported from linux-next (what will become linux 4.0) using > scripts/update-linux-headers.sh > > Signed-off-by: Michael S. Tsirkin > --- > include/standard-headers/sys/if_ether.h | 1 + > include/standard-headers/sys/types.h | 2 + > include/standard-headers/sys/virtio_9p.h | 44 + > include/standard-headers/sys/virtio_balloon.h | 59 +++ > include/standard-headers/sys/virtio_blk.h | 143 > include/standard-headers/sys/virtio_config.h | 64 +++ > include/standard-headers/sys/virtio_console.h | 78 + > include/standard-headers/sys/virtio_ids.h | 43 + > include/standard-headers/sys/virtio_net.h | 233 > ++ > include/standard-headers/sys/virtio_pci.h | 193 + > include/standard-headers/sys/virtio_ring.h| 171 +++ > include/standard-headers/sys/virtio_rng.h | 8 + > include/standard-headers/sys/virtio_scsi.h| 164 ++ > include/standard-headers/sys/virtio_types.h | 46 + > 14 files changed, 1249 insertions(+) > create mode 100644 include/standard-headers/sys/if_ether.h > create mode 100644 include/standard-headers/sys/types.h > create mode 100644 include/standard-headers/sys/virtio_9p.h > create mode 100644 include/standard-headers/sys/virtio_balloon.h > create mode 100644 include/standard-headers/sys/virtio_blk.h > create mode 100644 include/standard-headers/sys/virtio_config.h > create mode 100644 include/standard-headers/sys/virtio_console.h > create mode 100644 include/standard-headers/sys/virtio_ids.h > create mode 100644 include/standard-headers/sys/virtio_net.h > create mode 100644 include/standard-headers/sys/virtio_pci.h > create mode 100644 include/standard-headers/sys/virtio_ring.h > create mode 100644 include/standard-headers/sys/virtio_rng.h > create mode 100644 include/standard-headers/sys/virtio_scsi.h > create mode 100644 include/standard-headers/sys/virtio_types.h > > diff --git a/include/standard-headers/sys/if_ether.h > b/include/standard-headers/sys/if_ether.h > new file mode 100644 > index 000..91cf735 > --- /dev/null > +++ b/include/standard-headers/sys/if_ether.h > @@ -0,0 +1 @@ > +#define ETH_ALEN6 > diff --git a/include/standard-headers/sys/types.h > b/include/standard-headers/sys/types.h > new file mode 100644 > index 000..7d42ac6 > --- /dev/null > +++ b/include/standard-headers/sys/types.h > @@ -0,0 +1,2 @@ > +#include > +#include "qemu/compiler.h" > diff --git a/include/standard-headers/sys/virtio_9p.h > b/include/standard-headers/sys/virtio_9p.h > new file mode 100644 > index 000..19df0e4 > --- /dev/null > +++ b/include/standard-headers/sys/virtio_9p.h > @@ -0,0 +1,44 @@ > +#ifndef _LINUX_VIRTIO_9P_H > +#define _LINUX_VIRTIO_9P_H Would it make sense to replace the _LINUX_ in the header guards with the update-linux-headers.sh script, too (to avoid confusion with the real Linux headers)? Thomas
[Qemu-devel] [PATCH 3/4] util: Remove unused functions
Delete the unused functions qemu_opt_get_number_del(),
qemu_signalfd_available(), qemu_send_full() and qemu_recv_full().
Signed-off-by: Thomas Huth
---
include/qemu-common.h |4 ---
include/qemu/compatfd.h |1 -
include/qemu/option.h |2 -
util/compatfd.c | 19 -
util/osdep.c| 66 ---
util/qemu-option.c |6
6 files changed, 0 insertions(+), 98 deletions(-)
diff --git a/include/qemu-common.h b/include/qemu-common.h
index 644b46d..0aac082 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -217,10 +217,6 @@ void *qemu_oom_check(void *ptr);
ssize_t qemu_write_full(int fd, const void *buf, size_t count)
QEMU_WARN_UNUSED_RESULT;
-ssize_t qemu_send_full(int fd, const void *buf, size_t count, int flags)
-QEMU_WARN_UNUSED_RESULT;
-ssize_t qemu_recv_full(int fd, void *buf, size_t count, int flags)
-QEMU_WARN_UNUSED_RESULT;
#ifndef _WIN32
int qemu_pipe(int pipefd[2]);
diff --git a/include/qemu/compatfd.h b/include/qemu/compatfd.h
index 6b04877..fc37915 100644
--- a/include/qemu/compatfd.h
+++ b/include/qemu/compatfd.h
@@ -39,6 +39,5 @@ struct qemu_signalfd_siginfo {
};
int qemu_signalfd(const sigset_t *mask);
-bool qemu_signalfd_available(void);
#endif
diff --git a/include/qemu/option.h b/include/qemu/option.h
index 58c0157..9fdec02 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -89,8 +89,6 @@ bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool
defval);
uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t
defval);
uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t defval);
bool qemu_opt_get_bool_del(QemuOpts *opts, const char *name, bool defval);
-uint64_t qemu_opt_get_number_del(QemuOpts *opts, const char *name,
- uint64_t defval);
uint64_t qemu_opt_get_size_del(QemuOpts *opts, const char *name,
uint64_t defval);
int qemu_opt_unset(QemuOpts *opts, const char *name);
diff --git a/util/compatfd.c b/util/compatfd.c
index 341ada6..e857150 100644
--- a/util/compatfd.c
+++ b/util/compatfd.c
@@ -108,22 +108,3 @@ int qemu_signalfd(const sigset_t *mask)
return qemu_signalfd_compat(mask);
}
-
-bool qemu_signalfd_available(void)
-{
-#ifdef CONFIG_SIGNALFD
-sigset_t mask;
-int fd;
-bool ok;
-sigemptyset(&mask);
-errno = 0;
-fd = syscall(SYS_signalfd, -1, &mask, _NSIG / 8);
-ok = (errno != ENOSYS);
-if (fd >= 0) {
-close(fd);
-}
-return ok;
-#else
-return false;
-#endif
-}
diff --git a/util/osdep.c b/util/osdep.c
index b2bd154..f938b69 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -310,72 +310,6 @@ int qemu_accept(int s, struct sockaddr *addr, socklen_t
*addrlen)
return ret;
}
-/*
- * A variant of send(2) which handles partial write.
- *
- * Return the number of bytes transferred, which is only
- * smaller than `count' if there is an error.
- *
- * This function won't work with non-blocking fd's.
- * Any of the possibilities with non-bloking fd's is bad:
- * - return a short write (then name is wrong)
- * - busy wait adding (errno == EAGAIN) to the loop
- */
-ssize_t qemu_send_full(int fd, const void *buf, size_t count, int flags)
-{
-ssize_t ret = 0;
-ssize_t total = 0;
-
-while (count) {
-ret = send(fd, buf, count, flags);
-if (ret < 0) {
-if (errno == EINTR) {
-continue;
-}
-break;
-}
-
-count -= ret;
-buf += ret;
-total += ret;
-}
-
-return total;
-}
-
-/*
- * A variant of recv(2) which handles partial write.
- *
- * Return the number of bytes transferred, which is only
- * smaller than `count' if there is an error.
- *
- * This function won't work with non-blocking fd's.
- * Any of the possibilities with non-bloking fd's is bad:
- * - return a short write (then name is wrong)
- * - busy wait adding (errno == EAGAIN) to the loop
- */
-ssize_t qemu_recv_full(int fd, void *buf, size_t count, int flags)
-{
-ssize_t ret = 0;
-ssize_t total = 0;
-
-while (count) {
-ret = qemu_recv(fd, buf, count, flags);
-if (ret <= 0) {
-if (ret < 0 && errno == EINTR) {
-continue;
-}
-break;
-}
-
-count -= ret;
-buf += ret;
-total += ret;
-}
-
-return total;
-}
-
void qemu_set_version(const char *version)
{
qemu_version = version;
diff --git a/util/qemu-option.c b/util/qemu-option.c
index d3ab65d..0e408eb 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -433,12 +433,6 @@ uint64_t qemu_opt_get_number(QemuOpts *opts, const char
*name, uint64_t defval)
return qemu_opt_get_number_helper(opts, name, defval, false);
}
[Qemu-devel] [PATCH 1/4] migration: Remove unused functions
dup_mig_bytes_transferred(), skipped_mig_bytes_transferred(),
migrate_rdma_pin_all(), qsb_clone() and qsb_set_length()
are completely unused and thus can be deleted.
Signed-off-by: Thomas Huth
Cc: Juan Quintela
Cc: Amit Shah
---
arch_init.c | 10 ---
include/migration/migration.h |3 --
include/migration/qemu-file.h |2 -
migration/migration.c |9 ---
migration/qemu-file-buf.c | 53 -
5 files changed, 0 insertions(+), 77 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 89c8fa4..ad5ce28 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -249,21 +249,11 @@ static void acct_clear(void)
memset(&acct_info, 0, sizeof(acct_info));
}
-uint64_t dup_mig_bytes_transferred(void)
-{
-return acct_info.dup_pages * TARGET_PAGE_SIZE;
-}
-
uint64_t dup_mig_pages_transferred(void)
{
return acct_info.dup_pages;
}
-uint64_t skipped_mig_bytes_transferred(void)
-{
-return acct_info.skipped_pages * TARGET_PAGE_SIZE;
-}
-
uint64_t skipped_mig_pages_transferred(void)
{
return acct_info.skipped_pages;
diff --git a/include/migration/migration.h b/include/migration/migration.h
index f37348b..7f6cdaa 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -115,9 +115,7 @@ void free_xbzrle_decoded_buf(void);
void acct_update_position(QEMUFile *f, size_t size, bool zero);
-uint64_t dup_mig_bytes_transferred(void);
uint64_t dup_mig_pages_transferred(void);
-uint64_t skipped_mig_bytes_transferred(void);
uint64_t skipped_mig_pages_transferred(void);
uint64_t norm_mig_bytes_transferred(void);
uint64_t norm_mig_pages_transferred(void);
@@ -143,7 +141,6 @@ void migrate_add_blocker(Error *reason);
*/
void migrate_del_blocker(Error *reason);
-bool migrate_rdma_pin_all(void);
bool migrate_zero_blocks(void);
bool migrate_auto_converge(void);
diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
index a923cec..45d7f71 100644
--- a/include/migration/qemu-file.h
+++ b/include/migration/qemu-file.h
@@ -133,9 +133,7 @@ bool qemu_file_mode_is_not_valid(const char *mode);
bool qemu_file_is_writable(QEMUFile *f);
QEMUSizedBuffer *qsb_create(const uint8_t *buffer, size_t len);
-QEMUSizedBuffer *qsb_clone(const QEMUSizedBuffer *);
void qsb_free(QEMUSizedBuffer *);
-size_t qsb_set_length(QEMUSizedBuffer *qsb, size_t length);
size_t qsb_get_length(const QEMUSizedBuffer *qsb);
ssize_t qsb_get_buffer(const QEMUSizedBuffer *, off_t start, size_t count,
uint8_t *buf);
diff --git a/migration/migration.c b/migration/migration.c
index b3adbc6..6f1a490 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -540,15 +540,6 @@ void qmp_migrate_set_downtime(double value, Error **errp)
max_downtime = (uint64_t)value;
}
-bool migrate_rdma_pin_all(void)
-{
-MigrationState *s;
-
-s = migrate_get_current();
-
-return s->enabled_capabilities[MIGRATION_CAPABILITY_RDMA_PIN_ALL];
-}
-
bool migrate_auto_converge(void)
{
MigrationState *s;
diff --git a/migration/qemu-file-buf.c b/migration/qemu-file-buf.c
index e97e0bd..3b79c09 100644
--- a/migration/qemu-file-buf.c
+++ b/migration/qemu-file-buf.c
@@ -124,28 +124,6 @@ size_t qsb_get_length(const QEMUSizedBuffer *qsb)
}
/**
- * Set the length of the buffer; the primary usage of this
- * function is to truncate the number of used bytes in the buffer.
- * The size will not be extended beyond the current number of
- * allocated bytes in the QEMUSizedBuffer.
- *
- * @qsb: A QEMUSizedBuffer
- * @new_len: The new length of bytes in the buffer
- *
- * Returns the number of bytes the buffer was truncated or extended
- * to.
- */
-size_t qsb_set_length(QEMUSizedBuffer *qsb, size_t new_len)
-{
-if (new_len <= qsb->size) {
-qsb->used = new_len;
-} else {
-qsb->used = qsb->size;
-}
-return qsb->used;
-}
-
-/**
* Get the iovec that holds the data for a given position @pos.
*
* @qsb: A QEMUSizedBuffer
@@ -361,37 +339,6 @@ ssize_t qsb_write_at(QEMUSizedBuffer *qsb, const uint8_t
*source,
return count;
}
-/**
- * Create a deep copy of the given QEMUSizedBuffer.
- *
- * @qsb: A QEMUSizedBuffer
- *
- * Returns a clone of @qsb or NULL on allocation failure
- */
-QEMUSizedBuffer *qsb_clone(const QEMUSizedBuffer *qsb)
-{
-QEMUSizedBuffer *out = qsb_create(NULL, qsb_get_length(qsb));
-size_t i;
-ssize_t res;
-off_t pos = 0;
-
-if (!out) {
-return NULL;
-}
-
-for (i = 0; i < qsb->n_iov; i++) {
-res = qsb_write_at(out, qsb->iov[i].iov_base,
-pos, qsb->iov[i].iov_len);
-if (res < 0) {
-qsb_free(out);
-return NULL;
-}
-pos += res;
-}
-
-return out;
-}
-
typedef struct QEMUBuffer {
QEMUSizedBuffer *qsb;
QEMUFile *file;
--
1.7.1
[Qemu-devel] [PATCH 2/4] ui/console: Removed unused functions
Remove dpy_gfx_update_dirty(), qemu_console_get_head(),
qemu_console_get_ui_info(), qemu_console_get_width(),
qemu_console_get_height(), emu_console_displaystate(),
qemu_different_endianness_pixelformat(), void cpkey(),
qemu_pixman_linebuf_copy(), qemu_pixman_color(),
emu_remove_kbd_event_handler() and vnc_stop_worker_thread()
since they are completely unused.
Signed-off-by: Thomas Huth
Cc: Anthony Liguori
Cc: Gerd Hoffmann
---
include/ui/console.h | 11 -
include/ui/input.h |1 -
include/ui/qemu-pixman.h |3 -
ui/console.c | 103 --
ui/d3des.c |9
ui/d3des.h |6 ---
ui/input-legacy.c|6 ---
ui/input.c |7 ---
ui/qemu-pixman.c | 19
ui/vnc-jobs.c| 13 --
ui/vnc-jobs.h|1 -
11 files changed, 0 insertions(+), 179 deletions(-)
diff --git a/include/ui/console.h b/include/ui/console.h
index 8a4d671..abba208 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -36,7 +36,6 @@ typedef struct QEMUPutLEDEntry QEMUPutLEDEntry;
QEMUPutKbdEntry *qemu_add_kbd_event_handler(QEMUPutKBDEvent *func,
void *opaque);
-void qemu_remove_kbd_event_handler(QEMUPutKbdEntry *entry);
QEMUPutMouseEntry *qemu_add_mouse_event_handler(QEMUPutMouseEvent *func,
void *opaque, int absolute,
const char *name);
@@ -194,7 +193,6 @@ DisplaySurface *qemu_create_displaysurface_guestmem(int
width, int height,
pixman_format_code_t
format,
int linesize,
uint64_t addr);
-PixelFormat qemu_different_endianness_pixelformat(int bpp);
PixelFormat qemu_default_pixelformat(int bpp);
DisplaySurface *qemu_create_displaysurface(int width, int height);
@@ -233,10 +231,6 @@ void dpy_text_resize(QemuConsole *con, int w, int h);
void dpy_mouse_set(QemuConsole *con, int x, int y, int on);
void dpy_cursor_define(QemuConsole *con, QEMUCursor *cursor);
bool dpy_cursor_define_supported(QemuConsole *con);
-void dpy_gfx_update_dirty(QemuConsole *con,
- MemoryRegion *address_space,
- uint64_t base,
- bool invalidate);
bool dpy_gfx_check_format(QemuConsole *con,
pixman_format_code_t format);
@@ -310,10 +304,6 @@ bool qemu_console_is_visible(QemuConsole *con);
bool qemu_console_is_graphic(QemuConsole *con);
bool qemu_console_is_fixedsize(QemuConsole *con);
int qemu_console_get_index(QemuConsole *con);
-uint32_t qemu_console_get_head(QemuConsole *con);
-QemuUIInfo *qemu_console_get_ui_info(QemuConsole *con);
-int qemu_console_get_width(QemuConsole *con, int fallback);
-int qemu_console_get_height(QemuConsole *con, int fallback);
void text_consoles_set_display(DisplayState *ds);
void console_select(unsigned int index);
@@ -322,7 +312,6 @@ void qemu_console_resize(QemuConsole *con, int width, int
height);
void qemu_console_copy(QemuConsole *con, int src_x, int src_y,
int dst_x, int dst_y, int w, int h);
DisplaySurface *qemu_console_surface(QemuConsole *con);
-DisplayState *qemu_console_displaystate(QemuConsole *console);
/* sdl.c */
void sdl_display_init(DisplayState *ds, int full_screen, int no_frame);
diff --git a/include/ui/input.h b/include/ui/input.h
index 5d5ac00..d50ea9d 100644
--- a/include/ui/input.h
+++ b/include/ui/input.h
@@ -27,7 +27,6 @@ struct QemuInputHandler {
QemuInputHandlerState *qemu_input_handler_register(DeviceState *dev,
QemuInputHandler *handler);
void qemu_input_handler_activate(QemuInputHandlerState *s);
-void qemu_input_handler_deactivate(QemuInputHandlerState *s);
void qemu_input_handler_unregister(QemuInputHandlerState *s);
void qemu_input_handler_bind(QemuInputHandlerState *s,
const char *device_id, int head,
diff --git a/include/ui/qemu-pixman.h b/include/ui/qemu-pixman.h
index 3dee576..29c575c 100644
--- a/include/ui/qemu-pixman.h
+++ b/include/ui/qemu-pixman.h
@@ -44,13 +44,10 @@ pixman_image_t
*qemu_pixman_linebuf_create(pixman_format_code_t format,
int width);
void qemu_pixman_linebuf_fill(pixman_image_t *linebuf, pixman_image_t *fb,
int width, int x, int y);
-void qemu_pixman_linebuf_copy(pixman_image_t *fb, int width, int x, int y,
- pixman_image_t *linebuf);
pixman_image_t *qemu_pixman_mirror_create(pixman_format_code_t format,
pixman_image_t *image);
void qemu_pixman_image_unref(pixman_image_t *image);
-pixman_color_t qemu_pixman_color
[Qemu-devel] [PATCH 4/4] block: Remove unused functions
qemu_try_blockalign0() and nbd_export_close_all() are not
used anymore and thus can be removed.
Signed-off-by: Thomas Huth
Cc: Kevin Wolf
Cc: Stefan Hajnoczi
Cc: Paolo Bonzini
---
block.c | 11 ---
include/block/block.h |1 -
include/block/nbd.h |1 -
nbd.c |9 -
4 files changed, 0 insertions(+), 22 deletions(-)
diff --git a/block.c b/block.c
index 210fd5f..2128f82 100644
--- a/block.c
+++ b/block.c
@@ -5335,17 +5335,6 @@ void *qemu_try_blockalign(BlockDriverState *bs, size_t
size)
return qemu_try_memalign(align, size);
}
-void *qemu_try_blockalign0(BlockDriverState *bs, size_t size)
-{
-void *mem = qemu_try_blockalign(bs, size);
-
-if (mem) {
-memset(mem, 0, size);
-}
-
-return mem;
-}
-
/*
* Check if all memory in this vector is sector aligned.
*/
diff --git a/include/block/block.h b/include/block/block.h
index 321295e..7ee9ab3 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -433,7 +433,6 @@ void bdrv_set_guest_block_size(BlockDriverState *bs, int
align);
void *qemu_blockalign(BlockDriverState *bs, size_t size);
void *qemu_blockalign0(BlockDriverState *bs, size_t size);
void *qemu_try_blockalign(BlockDriverState *bs, size_t size);
-void *qemu_try_blockalign0(BlockDriverState *bs, size_t size);
bool bdrv_qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov);
struct HBitmapIter;
diff --git a/include/block/nbd.h b/include/block/nbd.h
index b759595..fccef2d 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -95,7 +95,6 @@ BlockBackend *nbd_export_get_blockdev(NBDExport *exp);
NBDExport *nbd_export_find(const char *name);
void nbd_export_set_name(NBDExport *exp, const char *name);
-void nbd_export_close_all(void);
NBDClient *nbd_client_new(NBDExport *exp, int csock,
void (*close)(NBDClient *));
diff --git a/nbd.c b/nbd.c
index e56afbc..634bf84 100644
--- a/nbd.c
+++ b/nbd.c
@@ -1067,15 +1067,6 @@ BlockBackend *nbd_export_get_blockdev(NBDExport *exp)
return exp->blk;
}
-void nbd_export_close_all(void)
-{
-NBDExport *exp, *next;
-
-QTAILQ_FOREACH_SAFE(exp, &exports, next, next) {
-nbd_export_close(exp);
-}
-}
-
static ssize_t nbd_co_send_reply(NBDRequest *req, struct nbd_reply *reply,
int len)
{
--
1.7.1
[Qemu-devel] [PATCH 0/4] Remove unused functions
There are quite a lot of completely unused functions scattered around in the QEMU sources - here are some patches to remove at least some of them. Thomas Huth (4): migration: Remove unused functions ui/console: Removed unused functions util: Remove unused functions block: Remove unused functions arch_init.c | 10 block.c | 11 include/block/block.h |1 - include/block/nbd.h |1 - include/migration/migration.h |3 - include/migration/qemu-file.h |2 - include/qemu-common.h |4 -- include/qemu/compatfd.h |1 - include/qemu/option.h |2 - include/ui/console.h | 11 include/ui/input.h|1 - include/ui/qemu-pixman.h |3 - migration/migration.c |9 migration/qemu-file-buf.c | 53 - nbd.c |9 ui/console.c | 103 - ui/d3des.c|9 ui/d3des.h|6 -- ui/input-legacy.c |6 -- ui/input.c|7 --- ui/qemu-pixman.c | 19 ui/vnc-jobs.c | 13 - ui/vnc-jobs.h |1 - util/compatfd.c | 19 util/osdep.c | 66 -- util/qemu-option.c|6 -- 26 files changed, 0 insertions(+), 376 deletions(-)
Re: [Qemu-devel] [PATCH 4/4] block: Remove unused functions
Hi Max, On Mon, 16 Feb 2015 17:26:56 -0500 Max Reitz wrote: > On 2015-02-16 at 16:41, Thomas Huth wrote: > > qemu_try_blockalign0() and nbd_export_close_all() are not > > used anymore and thus can be removed. > > > > Signed-off-by: Thomas Huth > > Cc: Kevin Wolf > > Cc: Stefan Hajnoczi > > Cc: Paolo Bonzini > > --- > > block.c | 11 --- > > include/block/block.h |1 - > > include/block/nbd.h |1 - > > nbd.c |9 - > > 4 files changed, 0 insertions(+), 22 deletions(-) > > NACK, I'm using nbd_export_close_all() in my "block: Rework > bdrv_close_all()" series. Alright, if it's going to be used again, then it must not be removed, of course. > I'm not so sure about qemu_try_blockalign0(); it has never been used, > but I introduced it because we have qemu_blockalign(), > qemu_try_blockalign(), and qemu_blockalign0() (the latter of which I > introduced along with qemu_try_blockalign0(), and this function is used). > > So I'd be fine with removing qemu_try_blockalign0() again, but I don't > really see the point in doing so. It is not a function that is per-se > deprecated or something, quite the opposite, actually. If people can > make use of that function, they should most certainly do so. I'm not a big fan of providing unused (and likely untested) functions just because they could be used somewhen in a distant future (unless they are part of a proper library) ... I'd rather add such functions again when they are really being used. Anyway, this is just a small, trivial function, and if you prefer to keep it, then please ignore this patch simply completely. There is certainly no urgent need for removing unused functions, I just wanted to make people aware that there are some. Thomas
Re: [Qemu-devel] [PATCH 3/4] util: Remove unused functions
Hi, On Tue, 17 Feb 2015 09:55:33 +0100 Markus Armbruster wrote: > Thomas Huth writes: > > > Delete the unused functions qemu_opt_get_number_del(), > > qemu_signalfd_available(), qemu_send_full() and qemu_recv_full(). > > > > Signed-off-by: Thomas Huth > > qemu_opt_get_number_del() was added in commit 782730b along with > qemu_opt_get_del(), qemu_opt_get_bool_del() and qemu_opt_get_size_del(). > It hasn't been used so far, but removing it makes the interface > irregular. I'd rather not. Ok, then let's keep them. > qemu_signalfd_available() is unused since commit 6d32717. It's also the > only user of CONFIG_SIGNALFD. If we want to drop it, I recommend > dropping CONFIG_SIGNALFD as well. As far as I can see, CONFIG_SIGNALFD is still used in qemu_signalfd(), so I think we've got to keep that. Thomas
Re: [Qemu-devel] [PATCH v3 03/17] virtio: use standard virtio_ring.h
On Mon, 16 Feb 2015 22:35:40 +0100
"Michael S. Tsirkin" wrote:
> Switch to virtio_ring.h from standard headers.
>
> Signed-off-by: Michael S. Tsirkin
> ---
> include/hw/virtio/dataplane/vring.h | 2 +-
> include/hw/virtio/virtio_ring.h | 167
>
> hw/net/vhost_net.c | 2 +-
> 3 files changed, 2 insertions(+), 169 deletions(-)
> delete mode 100644 include/hw/virtio/virtio_ring.h
>
> diff --git a/include/hw/virtio/dataplane/vring.h
> b/include/hw/virtio/dataplane/vring.h
> index d3e086a..836d26a 100644
> --- a/include/hw/virtio/dataplane/vring.h
> +++ b/include/hw/virtio/dataplane/vring.h
> @@ -18,7 +18,7 @@
> #define VRING_H
>
> #include "qemu-common.h"
> -#include "hw/virtio/virtio_ring.h"
> +#include "standard-headers/linux/virtio_ring.h"
> #include "hw/virtio/virtio.h"
>
> typedef struct {
> diff --git a/include/hw/virtio/virtio_ring.h b/include/hw/virtio/virtio_ring.h
> deleted file mode 100644
> index 0b42e6e..000
> --- a/include/hw/virtio/virtio_ring.h
> +++ /dev/null
> @@ -1,167 +0,0 @@
> -#ifndef _LINUX_VIRTIO_RING_H
> -#define _LINUX_VIRTIO_RING_H
> -/*
> - * This file is copied from /usr/include/linux while converting __uNN types
> - * to uXX_t, __inline__ to inline, and tab to spaces.
> - * */
Hmmm, in the old header, the "__inline__" was replaced with
"inline" ... in the new header from patch 2, there's now "__inline__"
again. Was there a good reason for this replacement? If yes, your
update-linux-headers.sh patch should maybe replace the "__inline__"
into "inline", too?
Apart from that, the patch looks good to me.
Thomas
Re: [Qemu-devel] [PATCH v3 02/17] include: import virtio headers from linux 4.0
On Mon, 16 Feb 2015 22:35:31 +0100 "Michael S. Tsirkin" wrote: > Add files imported from linux-next (what will become linux 4.0) using > scripts/update-linux-headers.sh > > Signed-off-by: Michael S. Tsirkin > --- > include/standard-headers/linux/if_ether.h | 1 + > include/standard-headers/linux/types.h | 2 + > include/standard-headers/linux/virtio_9p.h | 44 + > include/standard-headers/linux/virtio_balloon.h | 59 ++ > include/standard-headers/linux/virtio_blk.h | 143 +++ > include/standard-headers/linux/virtio_config.h | 64 +++ > include/standard-headers/linux/virtio_console.h | 78 > include/standard-headers/linux/virtio_ids.h | 43 + > include/standard-headers/linux/virtio_net.h | 233 > > include/standard-headers/linux/virtio_pci.h | 193 > include/standard-headers/linux/virtio_ring.h| 171 + > include/standard-headers/linux/virtio_rng.h | 8 + > include/standard-headers/linux/virtio_scsi.h| 164 + > include/standard-headers/linux/virtio_types.h | 46 + > 14 files changed, 1249 insertions(+) > create mode 100644 include/standard-headers/linux/if_ether.h > create mode 100644 include/standard-headers/linux/types.h > create mode 100644 include/standard-headers/linux/virtio_9p.h > create mode 100644 include/standard-headers/linux/virtio_balloon.h > create mode 100644 include/standard-headers/linux/virtio_blk.h > create mode 100644 include/standard-headers/linux/virtio_config.h > create mode 100644 include/standard-headers/linux/virtio_console.h > create mode 100644 include/standard-headers/linux/virtio_ids.h > create mode 100644 include/standard-headers/linux/virtio_net.h > create mode 100644 include/standard-headers/linux/virtio_pci.h > create mode 100644 include/standard-headers/linux/virtio_ring.h > create mode 100644 include/standard-headers/linux/virtio_rng.h > create mode 100644 include/standard-headers/linux/virtio_scsi.h > create mode 100644 include/standard-headers/linux/virtio_types.h > > diff --git a/include/standard-headers/linux/if_ether.h > b/include/standard-headers/linux/if_ether.h > new file mode 100644 > index 000..91cf735 > --- /dev/null > +++ b/include/standard-headers/linux/if_ether.h > @@ -0,0 +1 @@ > +#define ETH_ALEN6 > diff --git a/include/standard-headers/linux/types.h > b/include/standard-headers/linux/types.h > new file mode 100644 > index 000..7d42ac6 > --- /dev/null > +++ b/include/standard-headers/linux/types.h > @@ -0,0 +1,2 @@ > +#include That should be now, shouldn't it? Thomas
Re: [Qemu-devel] [PATCH v3 04/17] virtio: use standard-headers
) {
> @@ -218,7 +218,7 @@ void virtio_queue_set_notification(VirtQueue *vq, int
> enable)
> {
> vq->notification = enable;
> if (vq->vdev->guest_features & (1 << VIRTIO_RING_F_EVENT_IDX)) {
> -vring_avail_event(vq, vring_avail_idx(vq));
> +vring_set_avail_event(vq, vring_avail_idx(vq));
> } else if (enable) {
> vring_used_flags_unset_bit(vq, VRING_USED_F_NO_NOTIFY);
> } else {
> @@ -469,7 +469,7 @@ int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem)
>
> i = head = virtqueue_get_head(vq, vq->last_avail_idx++);
> if (vdev->guest_features & (1 << VIRTIO_RING_F_EVENT_IDX)) {
> -vring_avail_event(vq, vq->last_avail_idx);
> +vring_set_avail_event(vq, vq->last_avail_idx);
> }
>
> if (vring_desc_flags(vdev, desc_pa, i) & VRING_DESC_F_INDIRECT) {
> @@ -819,19 +819,6 @@ void virtio_irq(VirtQueue *vq)
> virtio_notify_vector(vq->vdev, vq->vector);
> }
>
> -/* Assuming a given event_idx value from the other size, if
> - * we have just incremented index from old to new_idx,
> - * should we trigger an event? */
> -static inline int vring_need_event(uint16_t event, uint16_t new, uint16_t
> old)
> -{
> - /* Note: Xen has similar logic for notification hold-off
> - * in include/xen/interface/io/ring.h with req_event and req_prod
> - * corresponding to event_idx + 1 and new respectively.
> - * Note also that req_event and req_prod in Xen start at 1,
> - * event indexes in virtio start at 0. */
> - return (uint16_t)(new - event - 1) < (uint16_t)(new - old);
> -}
> -
> static bool vring_notify(VirtIODevice *vdev, VirtQueue *vq)
> {
> uint16_t old, new;
> @@ -852,7 +839,7 @@ static bool vring_notify(VirtIODevice *vdev, VirtQueue
> *vq)
> vq->signalled_used_valid = true;
> old = vq->signalled_used;
> new = vq->signalled_used = vring_used_idx(vq);
> -return !v || vring_need_event(vring_used_event(vq), new, old);
> +return !v || vring_need_event(vring_get_used_event(vq), new, old);
> }
>
> void virtio_notify(VirtIODevice *vdev, VirtQueue *vq)
Reviewed-by: Thomas Huth
Re: [Qemu-devel] [PATCH v3 05/17] virtio-balloon: use standard headers
On Mon, 16 Feb 2015 22:35:52 +0100
"Michael S. Tsirkin" wrote:
> Drop code duplicated from standard headers.
>
> Signed-off-by: Michael S. Tsirkin
> ---
> include/hw/virtio/virtio-balloon.h | 35 ++-
> 1 file changed, 2 insertions(+), 33 deletions(-)
>
> diff --git a/include/hw/virtio/virtio-balloon.h
> b/include/hw/virtio/virtio-balloon.h
> index f863bfe..4ab8f54 100644
> --- a/include/hw/virtio/virtio-balloon.h
> +++ b/include/hw/virtio/virtio-balloon.h
> @@ -15,6 +15,7 @@
> #ifndef _QEMU_VIRTIO_BALLOON_H
> #define _QEMU_VIRTIO_BALLOON_H
>
> +#include "standard-headers/linux/virtio_balloon.h"
> #include "hw/virtio/virtio.h"
> #include "hw/pci/pci.h"
>
> @@ -22,39 +23,7 @@
> #define VIRTIO_BALLOON(obj) \
> OBJECT_CHECK(VirtIOBalloon, (obj), TYPE_VIRTIO_BALLOON)
>
> -/* from Linux's linux/virtio_balloon.h */
> -
> -/* The ID for virtio_balloon */
> -#define VIRTIO_ID_BALLOON 5
> -
> -/* The feature bitmap for virtio balloon */
> -#define VIRTIO_BALLOON_F_MUST_TELL_HOST 0 /* Tell before reclaiming pages */
> -#define VIRTIO_BALLOON_F_STATS_VQ 1 /* Memory stats virtqueue */
> -
> -/* Size of a PFN in the balloon interface. */
> -#define VIRTIO_BALLOON_PFN_SHIFT 12
> -
> -struct virtio_balloon_config
> -{
> -/* Number of pages host wants Guest to give up. */
> -uint32_t num_pages;
> -/* Number of pages we've actually got in balloon. */
> -uint32_t actual;
> -};
> -
> -/* Memory Statistics */
> -#define VIRTIO_BALLOON_S_SWAP_IN 0 /* Amount of memory swapped in */
> -#define VIRTIO_BALLOON_S_SWAP_OUT 1 /* Amount of memory swapped out */
> -#define VIRTIO_BALLOON_S_MAJFLT 2 /* Number of major faults */
> -#define VIRTIO_BALLOON_S_MINFLT 3 /* Number of minor faults */
> -#define VIRTIO_BALLOON_S_MEMFREE 4 /* Total amount of free memory */
> -#define VIRTIO_BALLOON_S_MEMTOT 5 /* Total amount of memory */
> -#define VIRTIO_BALLOON_S_NR 6
> -
> -typedef struct VirtIOBalloonStat {
> -uint16_t tag;
> -uint64_t val;
> -} QEMU_PACKED VirtIOBalloonStat;
> +typedef struct virtio_balloon_stat VirtIOBalloonStat;
>
> typedef struct VirtIOBalloon {
> VirtIODevice parent_obj;
Reviewed-by: Thomas Huth
Re: [Qemu-devel] [PATCH v3 06/17] virtio-9p: use standard headers
On Mon, 16 Feb 2015 22:35:57 +0100
"Michael S. Tsirkin" wrote:
> Drop code duplicated from standard headers.
>
> Signed-off-by: Michael S. Tsirkin
> ---
> hw/9pfs/virtio-9p.h | 17 +
> 1 file changed, 1 insertion(+), 16 deletions(-)
>
> diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
> index 0776424..58dafa9 100644
> --- a/hw/9pfs/virtio-9p.h
> +++ b/hw/9pfs/virtio-9p.h
> @@ -7,6 +7,7 @@
> #include
> #include
> #include
> +#include "standard-headers/linux/virtio_9p.h"
> #include "hw/virtio/virtio.h"
> #include "hw/virtio/virtio-9p.h"
> #include "fsdev/file-op-9p.h"
> @@ -14,10 +15,6 @@
> #include "qemu/thread.h"
> #include "block/coroutine.h"
>
> -/* The feature bitmap for virtio 9P */
> -/* The mount point is specified in a config variable */
> -#define VIRTIO_9P_MOUNT_TAG 0
> -
> enum {
> P9_TLERROR = 6,
> P9_RLERROR,
> @@ -145,10 +142,6 @@ struct V9fsPDU
> * 1) change user needs to set groups and stuff
> */
>
> -/* from Linux's linux/virtio_9p.h */
> -
> -/* The ID for virtio console */
> -#define VIRTIO_ID_9P9
> #define MAX_REQ 128
> #define MAX_TAG_LEN 32
>
> @@ -278,14 +271,6 @@ typedef struct V9fsWriteState {
> int cnt;
> } V9fsWriteState;
>
> -struct virtio_9p_config
> -{
> -/* number of characters in tag */
> -uint16_t tag_len;
> -/* Variable size tag name */
> -uint8_t tag[0];
> -} QEMU_PACKED;
> -
> typedef struct V9fsMkState {
> V9fsPDU *pdu;
> size_t offset;
Reviewed-by: Thomas Huth
Re: [Qemu-devel] [PATCH v3 07/17] virtio-blk: switch to standard-headers
e block device capacity is a multiple of
> * the logical block size. If that is not the case, let's use
> @@ -684,9 +684,9 @@ static void virtio_blk_update_config(VirtIODevice *vdev,
> uint8_t *config)
> * per track (cylinder).
> */
> if (blk_getlength(s->blk) / conf->heads / conf->secs % blk_size) {
> -blkcfg.sectors = conf->secs & ~s->sector_mask;
> +blkcfg.geometry.sectors = conf->secs & ~s->sector_mask;
> } else {
> -blkcfg.sectors = conf->secs;
> +blkcfg.geometry.sectors = conf->secs;
> }
> blkcfg.size_max = 0;
> blkcfg.physical_block_exp = get_physical_block_exp(conf);
Reviewed-by: Thomas Huth
Re: [Qemu-devel] [PATCH v3 08/17] virtio-net, tap: use standard-headers
3
> - #define VIRTIO_NET_CTRL_RX_NOUNI4
> - #define VIRTIO_NET_CTRL_RX_NOBCAST 5
> -
> -/*
> - * Control the MAC
> - *
> - * The MAC filter table is managed by the hypervisor, the guest should
> - * assume the size is infinite. Filtering should be considered
> - * non-perfect, ie. based on hypervisor resources, the guest may
> - * received packets from sources not specified in the filter list.
> - *
> - * In addition to the class/cmd header, the TABLE_SET command requires
> - * two out scatterlists. Each contains a 4 byte count of entries followed
> - * by a concatenated byte stream of the ETH_ALEN MAC addresses. The
> - * first sg list contains unicast addresses, the second is for multicast.
> - * This functionality is present if the VIRTIO_NET_F_CTRL_RX feature
> - * is available.
> - *
> - * The ADDR_SET command requests one out scatterlist, it contains a
> - * 6 bytes MAC address. This functionality is present if the
> - * VIRTIO_NET_F_CTRL_MAC_ADDR feature is available.
> - */
> -struct virtio_net_ctrl_mac {
> -uint32_t entries;
> -uint8_t macs[][ETH_ALEN];
> -};
> -
> typedef struct VirtIONetQueue {
> VirtQueue *rx_vq;
> VirtQueue *tx_vq;
> @@ -199,55 +99,6 @@ typedef struct VirtIONet {
> int announce_counter;
> } VirtIONet;
>
> -#define VIRTIO_NET_CTRL_MAC1
> - #define VIRTIO_NET_CTRL_MAC_TABLE_SET0
> - #define VIRTIO_NET_CTRL_MAC_ADDR_SET 1
> -
> -/*
> - * Control VLAN filtering
> - *
> - * The VLAN filter table is controlled via a simple ADD/DEL interface.
> - * VLAN IDs not added may be filterd by the hypervisor. Del is the
> - * opposite of add. Both commands expect an out entry containing a 2
> - * byte VLAN ID. VLAN filterting is available with the
> - * VIRTIO_NET_F_CTRL_VLAN feature bit.
> - */
> -#define VIRTIO_NET_CTRL_VLAN 2
> - #define VIRTIO_NET_CTRL_VLAN_ADD 0
> - #define VIRTIO_NET_CTRL_VLAN_DEL 1
> -
> -/*
> - * Control link announce acknowledgement
> - *
> - * VIRTIO_NET_S_ANNOUNCE bit in the status field requests link announcement
> from
> - * guest driver. The driver is notified by config space change interrupt.
> The
> - * command VIRTIO_NET_CTRL_ANNOUNCE_ACK is used to indicate that the driver
> has
> - * received the notification. It makes the device clear the bit
> - * VIRTIO_NET_S_ANNOUNCE in the status field.
> - */
> -#define VIRTIO_NET_CTRL_ANNOUNCE 3
> - #define VIRTIO_NET_CTRL_ANNOUNCE_ACK 0
> -
> -/*
> - * Control Multiqueue
> - *
> - * The command VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET
> - * enables multiqueue, specifying the number of the transmit and
> - * receive queues that will be used. After the command is consumed and acked
> by
> - * the device, the device will not steer new packets on receive virtqueues
> - * other than specified nor read from transmit virtqueues other than
> specified.
> - * Accordingly, driver should not transmit new packets on virtqueues other
> than
> - * specified.
> - */
> -struct virtio_net_ctrl_mq {
> -uint16_t virtqueue_pairs;
> -};
> -
> -#define VIRTIO_NET_CTRL_MQ 4
> - #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET0
> - #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN1
> - #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX0x8000
> -
> /*
> * Control network offloads
> *
> diff --git a/include/net/tap.h b/include/net/tap.h
> index 6daeb42..5da4edc 100644
> --- a/include/net/tap.h
> +++ b/include/net/tap.h
> @@ -28,6 +28,7 @@
>
> #include "qemu-common.h"
> #include "qapi-types.h"
> +#include "standard-headers/linux/virtio_net.h"
>
> int tap_enable(NetClientState *nc);
> int tap_disable(NetClientState *nc);
> @@ -37,27 +38,4 @@ int tap_get_fd(NetClientState *nc);
> struct vhost_net;
> struct vhost_net *tap_get_vhost_net(NetClientState *nc);
>
> -struct virtio_net_hdr
> -{
> -#define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 // Use csum_start,
> csum_offset
> -#define VIRTIO_NET_HDR_F_DATA_VALID2 // Csum is valid
> -uint8_t flags;
> -#define VIRTIO_NET_HDR_GSO_NONE 0 // Not a GSO frame
> -#define VIRTIO_NET_HDR_GSO_TCPV41 // GSO frame, IPv4 TCP (TSO)
> -#define VIRTIO_NET_HDR_GSO_UDP 3 // GSO frame, IPv4 UDP (UFO)
> -#define VIRTIO_NET_HDR_GSO_TCPV64 // GSO frame, IPv6 TCP
> -#define VIRTIO_NET_HDR_GSO_ECN 0x80// TCP has ECN set
> -uint8_t gso_type;
> -uint16_t hdr_len;
> -uint16_t gso_size;
> -uint16_t csum_start;
> -uint16_t csum_offset;
> -};
> -
> -struct virtio_net_hdr_mrg_rxbuf
> -{
> -struct virtio_net_hdr hdr;
> -uint16_t num_buffers; /* Number of merged rx buffers */
> -};
> -
> #endif /* QEMU_NET_TAP_H */
Reviewed-by: Thomas Huth
Re: [Qemu-devel] [PATCH v3 09/17] virtio-rng: use standard-headers
On Mon, 16 Feb 2015 22:36:15 +0100
"Michael S. Tsirkin" wrote:
> Drop duplicated code.
>
> Signed-off-by: Michael S. Tsirkin
> ---
> include/hw/virtio/virtio-rng.h | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/include/hw/virtio/virtio-rng.h b/include/hw/virtio/virtio-rng.h
> index 14e85a5..7702ff4 100644
> --- a/include/hw/virtio/virtio-rng.h
> +++ b/include/hw/virtio/virtio-rng.h
> @@ -14,6 +14,7 @@
>
> #include "sysemu/rng.h"
> #include "sysemu/rng-random.h"
> +#include "standard-headers/linux/virtio_rng.h"
>
> #define TYPE_VIRTIO_RNG "virtio-rng-device"
> #define VIRTIO_RNG(obj) \
> @@ -21,9 +22,6 @@
> #define VIRTIO_RNG_GET_PARENT_CLASS(obj) \
> OBJECT_GET_PARENT_CLASS(obj, TYPE_VIRTIO_RNG)
>
> -/* The Virtio ID for the virtio rng device */
> -#define VIRTIO_ID_RNG4
> -
> struct VirtIORNGConf {
> RngBackend *rng;
> uint64_t max_bytes;
Reviewed-by: Thomas Huth
Re: [Qemu-devel] [PATCH v3 11/17] virtio-serial: switch to standard-headers
On Mon, 16 Feb 2015 22:36:26 +0100
"Michael S. Tsirkin" wrote:
> Drop duplicate code.
>
> Signed-off-by: Michael S. Tsirkin
> ---
> include/hw/virtio/virtio-serial.h | 40
> +--
> hw/char/virtio-serial-bus.c | 1 +
> 2 files changed, 2 insertions(+), 39 deletions(-)
>
> diff --git a/include/hw/virtio/virtio-serial.h
> b/include/hw/virtio/virtio-serial.h
> index 11af978..ccf8459 100644
> --- a/include/hw/virtio/virtio-serial.h
> +++ b/include/hw/virtio/virtio-serial.h
> @@ -15,53 +15,15 @@
> #ifndef _QEMU_VIRTIO_SERIAL_H
> #define _QEMU_VIRTIO_SERIAL_H
>
> +#include "standard-headers/linux/virtio_console.h"
> #include "hw/qdev.h"
> #include "hw/virtio/virtio.h"
>
> -/* == Interface shared between the guest kernel and qemu == */
> -
> -/* The Virtio ID for virtio console / serial ports */
> -#define VIRTIO_ID_CONSOLE3
> -
> -/* Features supported */
> -#define VIRTIO_CONSOLE_F_MULTIPORT 1
> -
> -#define VIRTIO_CONSOLE_BAD_ID (~(uint32_t)0)
> -
> -struct virtio_console_config {
> -/*
> - * These two fields are used by VIRTIO_CONSOLE_F_SIZE which
> - * isn't implemented here yet
> - */
> -uint16_t cols;
> -uint16_t rows;
> -
> -uint32_t max_nr_ports;
> -} QEMU_PACKED;
> -
> -struct virtio_console_control {
> -uint32_t id; /* Port number */
> -uint16_t event; /* The kind of control event (see below) */
> -uint16_t value; /* Extra information for the key */
> -};
> -
> struct virtio_serial_conf {
> /* Max. number of ports we can have for a virtio-serial device */
> uint32_t max_virtserial_ports;
> };
>
> -/* Some events for the internal messages (control packets) */
> -#define VIRTIO_CONSOLE_DEVICE_READY 0
> -#define VIRTIO_CONSOLE_PORT_ADD 1
> -#define VIRTIO_CONSOLE_PORT_REMOVE 2
> -#define VIRTIO_CONSOLE_PORT_READY3
> -#define VIRTIO_CONSOLE_CONSOLE_PORT 4
> -#define VIRTIO_CONSOLE_RESIZE5
> -#define VIRTIO_CONSOLE_PORT_OPEN 6
> -#define VIRTIO_CONSOLE_PORT_NAME 7
> -
> -/* == In-qemu interface == */
> -
> #define TYPE_VIRTIO_SERIAL_PORT "virtio-serial-port"
> #define VIRTIO_SERIAL_PORT(obj) \
> OBJECT_CHECK(VirtIOSerialPort, (obj), TYPE_VIRTIO_SERIAL_PORT)
> diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
> index 47fbb34..a2bac9b 100644
> --- a/hw/char/virtio-serial-bus.c
> +++ b/hw/char/virtio-serial-bus.c
> @@ -18,6 +18,7 @@
> * GNU GPL, version 2 or (at your option) any later version.
> */
>
> +#include "standard-headers/linux/virtio_ids.h"
Could you omit this include? Later in virtio-serial-bus.c, the code
includes hw/virtio/virtio-serial.h, which in turn includes
standard-headers/linux/virtio_console.h - and that one already includes
the ids.h file. So I think it should work without the above change, too.
Thomas
Re: [Qemu-devel] [PATCH v3 11/17] virtio-serial: switch to standard-headers
On Wed, 18 Feb 2015 15:55:54 +0100 "Michael S. Tsirkin" wrote: > On Wed, Feb 18, 2015 at 03:34:13PM +0100, Thomas Huth wrote: > > On Mon, 16 Feb 2015 22:36:26 +0100 > > "Michael S. Tsirkin" wrote: ... > > > diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c > > > index 47fbb34..a2bac9b 100644 > > > --- a/hw/char/virtio-serial-bus.c > > > +++ b/hw/char/virtio-serial-bus.c > > > @@ -18,6 +18,7 @@ > > > * GNU GPL, version 2 or (at your option) any later version. > > > */ > > > > > > +#include "standard-headers/linux/virtio_ids.h" > > > > Could you omit this include? Later in virtio-serial-bus.c, the code > > includes hw/virtio/virtio-serial.h, which in turn includes > > standard-headers/linux/virtio_console.h - and that one already includes > > the ids.h file. So I think it should work without the above change, too. > > > > Thomas > > Yes but it's generally not a good idea to depend on headers > including each other. But as far as I can see, you also did not do this change in your other patches of this series, so this seems a little bit inconsequent (e.g. virtio-blk.c depends on VIRTIO_ID_BLOCK, but you did not include the virtio_ids.h header there). Thomas
Re: [Qemu-devel] [PATCH] KVM: s390: Add MEMOP ioctls for reading/writing guest memory
On Thu, 19 Feb 2015 10:47:29 +0100
Cornelia Huck wrote:
> On Mon, 16 Feb 2015 13:16:41 +0100
> Thomas Huth wrote:
>
> > On s390, we've got to make sure to hold the IPTE lock while accessing
> > logical memory. So let's add an ioctl for reading and writing logical
> > memory to provide this feature for userspace, too.
> >
> > Signed-off-by: Thomas Huth
> > ---
> > Documentation/virtual/kvm/api.txt | 45 +++
> > arch/s390/kvm/gaccess.c | 22 +++
> > arch/s390/kvm/gaccess.h |2 +
> > arch/s390/kvm/kvm-s390.c | 70
> > +
> > include/uapi/linux/kvm.h | 20 ++
> > 5 files changed, 159 insertions(+), 0 deletions(-)
>
> Looks good, except for one minor thing:
>
> > +static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
> > + struct kvm_s390_mem_op *mop)
> > +{
> > + void __user *uaddr = (void __user *)mop->buf;
> > + void *tmpbuf = NULL;
> > + int r, srcu_idx;
> > + const u64 supported_flags = KVM_S390_MEMOP_F_INJECT_EXCEPTION
> > + | KVM_S390_MEMOP_F_CHECK_ONLY;
> > +
> > + if (mop->flags & ~supported_flags)
> > + return -EINVAL;
> > +
> > + if (mop->size > 16 * PAGE_SIZE)
> > + return -E2BIG;
>
> Do you want to document this limit? Or make it discoverable by having
> the capability check return the number of pages?
I like the idea with the capability ... I think I'll add that feature.
Thomas
[Qemu-devel] [PATCH 2/5] ui: Removed unused functions
Remove qemu_console_displaystate(), qemu_remove_kbd_event_handler(),
qemu_different_endianness_pixelformat() and cpkey(), since they are
completely unused.
Signed-off-by: Thomas Huth
Cc: Anthony Liguori
Cc: Gerd Hoffmann
---
include/ui/console.h |3 ---
ui/console.c | 12
ui/d3des.c |9 -
ui/d3des.h |6 --
ui/input-legacy.c|6 --
5 files changed, 0 insertions(+), 36 deletions(-)
diff --git a/include/ui/console.h b/include/ui/console.h
index 8a4d671..7c3b5d4 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -36,7 +36,6 @@ typedef struct QEMUPutLEDEntry QEMUPutLEDEntry;
QEMUPutKbdEntry *qemu_add_kbd_event_handler(QEMUPutKBDEvent *func,
void *opaque);
-void qemu_remove_kbd_event_handler(QEMUPutKbdEntry *entry);
QEMUPutMouseEntry *qemu_add_mouse_event_handler(QEMUPutMouseEvent *func,
void *opaque, int absolute,
const char *name);
@@ -194,7 +193,6 @@ DisplaySurface *qemu_create_displaysurface_guestmem(int
width, int height,
pixman_format_code_t
format,
int linesize,
uint64_t addr);
-PixelFormat qemu_different_endianness_pixelformat(int bpp);
PixelFormat qemu_default_pixelformat(int bpp);
DisplaySurface *qemu_create_displaysurface(int width, int height);
@@ -322,7 +320,6 @@ void qemu_console_resize(QemuConsole *con, int width, int
height);
void qemu_console_copy(QemuConsole *con, int src_x, int src_y,
int dst_x, int dst_y, int w, int h);
DisplaySurface *qemu_console_surface(QemuConsole *con);
-DisplayState *qemu_console_displaystate(QemuConsole *console);
/* sdl.c */
void sdl_display_init(DisplayState *ds, int full_screen, int no_frame);
diff --git a/ui/console.c b/ui/console.c
index 87574a7..87af6b5 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -2005,18 +2005,6 @@ DisplaySurface *qemu_console_surface(QemuConsole
*console)
return console->surface;
}
-DisplayState *qemu_console_displaystate(QemuConsole *console)
-{
-return console->ds;
-}
-
-PixelFormat qemu_different_endianness_pixelformat(int bpp)
-{
-pixman_format_code_t fmt = qemu_default_pixman_format(bpp, false);
-PixelFormat pf = qemu_pixelformat_from_pixman(fmt);
-return pf;
-}
-
PixelFormat qemu_default_pixelformat(int bpp)
{
pixman_format_code_t fmt = qemu_default_pixman_format(bpp, true);
diff --git a/ui/d3des.c b/ui/d3des.c
index 60c840e..5bc99b8 100644
--- a/ui/d3des.c
+++ b/ui/d3des.c
@@ -121,15 +121,6 @@ static void cookey(register unsigned long *raw1)
return;
}
-void cpkey(register unsigned long *into)
-{
- register unsigned long *from, *endp;
-
- from = KnL, endp = &KnL[32];
- while( from < endp ) *into++ = *from++;
- return;
- }
-
void usekey(register unsigned long *from)
{
register unsigned long *to, *endp;
diff --git a/ui/d3des.h b/ui/d3des.h
index 70cb6b5..773667e 100644
--- a/ui/d3des.h
+++ b/ui/d3des.h
@@ -36,12 +36,6 @@ void usekey(unsigned long *);
* Loads the internal key register with the data in cookedkey.
*/
-void cpkey(unsigned long *);
-/*cookedkey[32]
- * Copies the contents of the internal key register into the storage
- * located at &cookedkey[0].
- */
-
void des(unsigned char *, unsigned char *);
/* from[8] to[8]
* Encrypts/Decrypts (according to the key currently loaded in the
diff --git a/ui/input-legacy.c b/ui/input-legacy.c
index a698a34..2d4ca19 100644
--- a/ui/input-legacy.c
+++ b/ui/input-legacy.c
@@ -143,12 +143,6 @@ QEMUPutKbdEntry
*qemu_add_kbd_event_handler(QEMUPutKBDEvent *func, void *opaque)
return entry;
}
-void qemu_remove_kbd_event_handler(QEMUPutKbdEntry *entry)
-{
-qemu_input_handler_unregister(entry->s);
-g_free(entry);
-}
-
static void legacy_mouse_event(DeviceState *dev, QemuConsole *src,
InputEvent *evt)
{
--
1.7.1
[Qemu-devel] [PATCH 3/5] ui/vnc: Remove vnc_stop_worker_thread()
This function is not used anymore, let's remove it.
Signed-off-by: Thomas Huth
Cc: Anthony Liguori
Cc: Gerd Hoffmann
---
ui/vnc-jobs.c | 13 -
ui/vnc-jobs.h |1 -
2 files changed, 0 insertions(+), 14 deletions(-)
diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
index 68f3d77..c8ee203 100644
--- a/ui/vnc-jobs.c
+++ b/ui/vnc-jobs.c
@@ -342,16 +342,3 @@ void vnc_start_worker_thread(void)
QEMU_THREAD_DETACHED);
queue = q; /* Set global queue */
}
-
-void vnc_stop_worker_thread(void)
-{
-if (!vnc_worker_thread_running())
-return ;
-
-/* Remove all jobs and wake up the thread */
-vnc_lock_queue(queue);
-queue->exit = true;
-vnc_unlock_queue(queue);
-vnc_jobs_clear(NULL);
-qemu_cond_broadcast(&queue->cond);
-}
diff --git a/ui/vnc-jobs.h b/ui/vnc-jobs.h
index 31da103..044bf9f 100644
--- a/ui/vnc-jobs.h
+++ b/ui/vnc-jobs.h
@@ -40,7 +40,6 @@ void vnc_jobs_join(VncState *vs);
void vnc_jobs_consume_buffer(VncState *vs);
void vnc_start_worker_thread(void);
-void vnc_stop_worker_thread(void);
/* Locks */
static inline int vnc_trylock_display(VncDisplay *vd)
--
1.7.1
[Qemu-devel] [PATCH 1/5] migration: Remove unused functions
dup_mig_bytes_transferred(), skipped_mig_bytes_transferred(),
migrate_rdma_pin_all(), qsb_clone() and qsb_set_length()
are completely unused and thus can be deleted.
Signed-off-by: Thomas Huth
Cc: Juan Quintela
Cc: Amit Shah
---
arch_init.c | 10 ---
include/migration/migration.h |3 --
include/migration/qemu-file.h |2 -
migration/migration.c |9 ---
migration/qemu-file-buf.c | 53 -
5 files changed, 0 insertions(+), 77 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 89c8fa4..ad5ce28 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -249,21 +249,11 @@ static void acct_clear(void)
memset(&acct_info, 0, sizeof(acct_info));
}
-uint64_t dup_mig_bytes_transferred(void)
-{
-return acct_info.dup_pages * TARGET_PAGE_SIZE;
-}
-
uint64_t dup_mig_pages_transferred(void)
{
return acct_info.dup_pages;
}
-uint64_t skipped_mig_bytes_transferred(void)
-{
-return acct_info.skipped_pages * TARGET_PAGE_SIZE;
-}
-
uint64_t skipped_mig_pages_transferred(void)
{
return acct_info.skipped_pages;
diff --git a/include/migration/migration.h b/include/migration/migration.h
index f37348b..7f6cdaa 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -115,9 +115,7 @@ void free_xbzrle_decoded_buf(void);
void acct_update_position(QEMUFile *f, size_t size, bool zero);
-uint64_t dup_mig_bytes_transferred(void);
uint64_t dup_mig_pages_transferred(void);
-uint64_t skipped_mig_bytes_transferred(void);
uint64_t skipped_mig_pages_transferred(void);
uint64_t norm_mig_bytes_transferred(void);
uint64_t norm_mig_pages_transferred(void);
@@ -143,7 +141,6 @@ void migrate_add_blocker(Error *reason);
*/
void migrate_del_blocker(Error *reason);
-bool migrate_rdma_pin_all(void);
bool migrate_zero_blocks(void);
bool migrate_auto_converge(void);
diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
index a923cec..45d7f71 100644
--- a/include/migration/qemu-file.h
+++ b/include/migration/qemu-file.h
@@ -133,9 +133,7 @@ bool qemu_file_mode_is_not_valid(const char *mode);
bool qemu_file_is_writable(QEMUFile *f);
QEMUSizedBuffer *qsb_create(const uint8_t *buffer, size_t len);
-QEMUSizedBuffer *qsb_clone(const QEMUSizedBuffer *);
void qsb_free(QEMUSizedBuffer *);
-size_t qsb_set_length(QEMUSizedBuffer *qsb, size_t length);
size_t qsb_get_length(const QEMUSizedBuffer *qsb);
ssize_t qsb_get_buffer(const QEMUSizedBuffer *, off_t start, size_t count,
uint8_t *buf);
diff --git a/migration/migration.c b/migration/migration.c
index b3adbc6..6f1a490 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -540,15 +540,6 @@ void qmp_migrate_set_downtime(double value, Error **errp)
max_downtime = (uint64_t)value;
}
-bool migrate_rdma_pin_all(void)
-{
-MigrationState *s;
-
-s = migrate_get_current();
-
-return s->enabled_capabilities[MIGRATION_CAPABILITY_RDMA_PIN_ALL];
-}
-
bool migrate_auto_converge(void)
{
MigrationState *s;
diff --git a/migration/qemu-file-buf.c b/migration/qemu-file-buf.c
index e97e0bd..3b79c09 100644
--- a/migration/qemu-file-buf.c
+++ b/migration/qemu-file-buf.c
@@ -124,28 +124,6 @@ size_t qsb_get_length(const QEMUSizedBuffer *qsb)
}
/**
- * Set the length of the buffer; the primary usage of this
- * function is to truncate the number of used bytes in the buffer.
- * The size will not be extended beyond the current number of
- * allocated bytes in the QEMUSizedBuffer.
- *
- * @qsb: A QEMUSizedBuffer
- * @new_len: The new length of bytes in the buffer
- *
- * Returns the number of bytes the buffer was truncated or extended
- * to.
- */
-size_t qsb_set_length(QEMUSizedBuffer *qsb, size_t new_len)
-{
-if (new_len <= qsb->size) {
-qsb->used = new_len;
-} else {
-qsb->used = qsb->size;
-}
-return qsb->used;
-}
-
-/**
* Get the iovec that holds the data for a given position @pos.
*
* @qsb: A QEMUSizedBuffer
@@ -361,37 +339,6 @@ ssize_t qsb_write_at(QEMUSizedBuffer *qsb, const uint8_t
*source,
return count;
}
-/**
- * Create a deep copy of the given QEMUSizedBuffer.
- *
- * @qsb: A QEMUSizedBuffer
- *
- * Returns a clone of @qsb or NULL on allocation failure
- */
-QEMUSizedBuffer *qsb_clone(const QEMUSizedBuffer *qsb)
-{
-QEMUSizedBuffer *out = qsb_create(NULL, qsb_get_length(qsb));
-size_t i;
-ssize_t res;
-off_t pos = 0;
-
-if (!out) {
-return NULL;
-}
-
-for (i = 0; i < qsb->n_iov; i++) {
-res = qsb_write_at(out, qsb->iov[i].iov_base,
-pos, qsb->iov[i].iov_len);
-if (res < 0) {
-qsb_free(out);
-return NULL;
-}
-pos += res;
-}
-
-return out;
-}
-
typedef struct QEMUBuffer {
QEMUSizedBuffer *qsb;
QEMUFile *file;
--
1.7.1
[Qemu-devel] [PATCH 5/5] xen: Remove xen_cmos_set_s3_resume()
The function is not used anymore, and thus can be deleted.
Signed-off-by: Thomas Huth
Cc: Stefano Stabellini
---
include/hw/xen/xen.h |1 -
xen-hvm-stub.c |4
2 files changed, 0 insertions(+), 5 deletions(-)
diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
index b0ed04c..4356af4 100644
--- a/include/hw/xen/xen.h
+++ b/include/hw/xen/xen.h
@@ -32,7 +32,6 @@ int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num);
void xen_piix3_set_irq(void *opaque, int irq_num, int level);
void xen_piix_pci_write_config_client(uint32_t address, uint32_t val, int len);
void xen_hvm_inject_msi(uint64_t addr, uint32_t data);
-void xen_cmos_set_s3_resume(void *opaque, int irq, int level);
qemu_irq *xen_interrupt_controller_init(void);
diff --git a/xen-hvm-stub.c b/xen-hvm-stub.c
index 2d98696..46867d8 100644
--- a/xen-hvm-stub.c
+++ b/xen-hvm-stub.c
@@ -30,10 +30,6 @@ void xen_hvm_inject_msi(uint64_t addr, uint32_t data)
{
}
-void xen_cmos_set_s3_resume(void *opaque, int irq, int level)
-{
-}
-
void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr)
{
}
--
1.7.1
[Qemu-devel] [PATCH 4/5] util: Remove unused functions
Delete the unused functions qemu_signalfd_available(),
qemu_send_full() and qemu_recv_full().
Signed-off-by: Thomas Huth
Cc: Markus Armbruster
---
include/qemu-common.h |4 ---
include/qemu/compatfd.h |1 -
util/compatfd.c | 19 -
util/osdep.c| 66 ---
4 files changed, 0 insertions(+), 90 deletions(-)
diff --git a/include/qemu-common.h b/include/qemu-common.h
index 644b46d..0aac082 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -217,10 +217,6 @@ void *qemu_oom_check(void *ptr);
ssize_t qemu_write_full(int fd, const void *buf, size_t count)
QEMU_WARN_UNUSED_RESULT;
-ssize_t qemu_send_full(int fd, const void *buf, size_t count, int flags)
-QEMU_WARN_UNUSED_RESULT;
-ssize_t qemu_recv_full(int fd, void *buf, size_t count, int flags)
-QEMU_WARN_UNUSED_RESULT;
#ifndef _WIN32
int qemu_pipe(int pipefd[2]);
diff --git a/include/qemu/compatfd.h b/include/qemu/compatfd.h
index 6b04877..fc37915 100644
--- a/include/qemu/compatfd.h
+++ b/include/qemu/compatfd.h
@@ -39,6 +39,5 @@ struct qemu_signalfd_siginfo {
};
int qemu_signalfd(const sigset_t *mask);
-bool qemu_signalfd_available(void);
#endif
diff --git a/util/compatfd.c b/util/compatfd.c
index 341ada6..e857150 100644
--- a/util/compatfd.c
+++ b/util/compatfd.c
@@ -108,22 +108,3 @@ int qemu_signalfd(const sigset_t *mask)
return qemu_signalfd_compat(mask);
}
-
-bool qemu_signalfd_available(void)
-{
-#ifdef CONFIG_SIGNALFD
-sigset_t mask;
-int fd;
-bool ok;
-sigemptyset(&mask);
-errno = 0;
-fd = syscall(SYS_signalfd, -1, &mask, _NSIG / 8);
-ok = (errno != ENOSYS);
-if (fd >= 0) {
-close(fd);
-}
-return ok;
-#else
-return false;
-#endif
-}
diff --git a/util/osdep.c b/util/osdep.c
index b2bd154..f938b69 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -310,72 +310,6 @@ int qemu_accept(int s, struct sockaddr *addr, socklen_t
*addrlen)
return ret;
}
-/*
- * A variant of send(2) which handles partial write.
- *
- * Return the number of bytes transferred, which is only
- * smaller than `count' if there is an error.
- *
- * This function won't work with non-blocking fd's.
- * Any of the possibilities with non-bloking fd's is bad:
- * - return a short write (then name is wrong)
- * - busy wait adding (errno == EAGAIN) to the loop
- */
-ssize_t qemu_send_full(int fd, const void *buf, size_t count, int flags)
-{
-ssize_t ret = 0;
-ssize_t total = 0;
-
-while (count) {
-ret = send(fd, buf, count, flags);
-if (ret < 0) {
-if (errno == EINTR) {
-continue;
-}
-break;
-}
-
-count -= ret;
-buf += ret;
-total += ret;
-}
-
-return total;
-}
-
-/*
- * A variant of recv(2) which handles partial write.
- *
- * Return the number of bytes transferred, which is only
- * smaller than `count' if there is an error.
- *
- * This function won't work with non-blocking fd's.
- * Any of the possibilities with non-bloking fd's is bad:
- * - return a short write (then name is wrong)
- * - busy wait adding (errno == EAGAIN) to the loop
- */
-ssize_t qemu_recv_full(int fd, void *buf, size_t count, int flags)
-{
-ssize_t ret = 0;
-ssize_t total = 0;
-
-while (count) {
-ret = qemu_recv(fd, buf, count, flags);
-if (ret <= 0) {
-if (ret < 0 && errno == EINTR) {
-continue;
-}
-break;
-}
-
-count -= ret;
-buf += ret;
-total += ret;
-}
-
-return total;
-}
-
void qemu_set_version(const char *version)
{
qemu_version = version;
--
1.7.1
[Qemu-devel] [PATCH v2 0/5] Remove unused functions
There are quite a lot of completely unused functions scattered around in the QEMU sources - here are some patches to remove at least some of them. v2: - Incorporated review feedback to keep the functions that still might be usefull (e.g. dropped the patch to remove unused functions in the block layer) - Added a patch to remove xen_cmos_set_s3_resume() Thomas Huth (5): migration: Remove unused functions ui: Removed unused functions ui/vnc: Remove vnc_stop_worker_thread() util: Remove unused functions xen: Remove xen_cmos_set_s3_resume() arch_init.c | 10 -- include/hw/xen/xen.h |1 - include/migration/migration.h |3 -- include/migration/qemu-file.h |2 - include/qemu-common.h |4 -- include/qemu/compatfd.h |1 - include/ui/console.h |3 -- migration/migration.c |9 - migration/qemu-file-buf.c | 53 - ui/console.c | 12 --- ui/d3des.c|9 - ui/d3des.h|6 ui/input-legacy.c |6 ui/vnc-jobs.c | 13 ui/vnc-jobs.h |1 - util/compatfd.c | 19 util/osdep.c | 66 - xen-hvm-stub.c|4 -- 18 files changed, 0 insertions(+), 222 deletions(-)
Re: [Qemu-devel] [PATCH 1/5] migration: Remove unused functions
On Tue, 24 Feb 2015 14:58:29 +0530
Amit Shah wrote:
> On (Thu) 19 Feb 2015 [18:12:19], Thomas Huth wrote:
> > dup_mig_bytes_transferred(), skipped_mig_bytes_transferred(),
> > migrate_rdma_pin_all(), qsb_clone() and qsb_set_length()
> > are completely unused and thus can be deleted.
> >
> > Signed-off-by: Thomas Huth
> > Cc: Juan Quintela
> > Cc: Amit Shah
> > ---
> > arch_init.c | 10 ---
> > include/migration/migration.h |3 --
> > include/migration/qemu-file.h |2 -
> > migration/migration.c |9 ---
> > migration/qemu-file-buf.c | 53
> > -
> > 5 files changed, 0 insertions(+), 77 deletions(-)
> >
> > diff --git a/arch_init.c b/arch_init.c
> > index 89c8fa4..ad5ce28 100644
> > --- a/arch_init.c
> > +++ b/arch_init.c
> > @@ -249,21 +249,11 @@ static void acct_clear(void)
> > memset(&acct_info, 0, sizeof(acct_info));
> > }
> >
> > -uint64_t dup_mig_bytes_transferred(void)
> > -{
> > -return acct_info.dup_pages * TARGET_PAGE_SIZE;
> > -}
> > -
> > uint64_t dup_mig_pages_transferred(void)
> > {
> > return acct_info.dup_pages;
> > }
> >
> > -uint64_t skipped_mig_bytes_transferred(void)
> > -{
> > -return acct_info.skipped_pages * TARGET_PAGE_SIZE;
> > -}
>
> These could be used for reporting; Juan, any idea why these aren't
> used?
Since they are very trivial, I think they could easily be re-added
again in case they are needed again in the future.
Or if you prefer, I can also rework my patch so that these two
functions won't get deleted.
Thomas
[Qemu-devel] [PATCH v3 0/3] Remove unused functions
There are quite a lot of completely unused functions scattered around in the QEMU sources - here are some patches to remove at least some of them. I've limited the patches now to the ones which have been ack'ed by the corresponding subsystem maintainers, so it should be really save now to apply these patches (I'll maybe re-post the other patches later again for further discusison). Michael, can you take these through your trivial-patches repository? Thomas Huth (3): ui: Removed unused functions ui/vnc: Remove vnc_stop_worker_thread() xen: Remove xen_cmos_set_s3_resume() include/hw/xen/xen.h |1 - include/ui/console.h |3 --- ui/console.c | 12 ui/d3des.c |9 - ui/d3des.h |6 -- ui/input-legacy.c|6 -- ui/vnc-jobs.c| 13 - ui/vnc-jobs.h|1 - xen-hvm-stub.c |4 9 files changed, 0 insertions(+), 55 deletions(-)
[Qemu-devel] [PATCH 1/3] ui: Removed unused functions
Remove qemu_console_displaystate(), qemu_remove_kbd_event_handler(),
qemu_different_endianness_pixelformat() and cpkey(), since they are
completely unused.
Signed-off-by: Thomas Huth
Reviewed-by: Gerd Hoffmann
---
include/ui/console.h |3 ---
ui/console.c | 12
ui/d3des.c |9 -
ui/d3des.h |6 --
ui/input-legacy.c|6 --
5 files changed, 0 insertions(+), 36 deletions(-)
diff --git a/include/ui/console.h b/include/ui/console.h
index 0f97d86..6e5a867 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -36,7 +36,6 @@ typedef struct QEMUPutLEDEntry QEMUPutLEDEntry;
QEMUPutKbdEntry *qemu_add_kbd_event_handler(QEMUPutKBDEvent *func,
void *opaque);
-void qemu_remove_kbd_event_handler(QEMUPutKbdEntry *entry);
QEMUPutMouseEntry *qemu_add_mouse_event_handler(QEMUPutMouseEvent *func,
void *opaque, int absolute,
const char *name);
@@ -194,7 +193,6 @@ DisplaySurface *qemu_create_displaysurface_guestmem(int
width, int height,
pixman_format_code_t
format,
int linesize,
uint64_t addr);
-PixelFormat qemu_different_endianness_pixelformat(int bpp);
PixelFormat qemu_default_pixelformat(int bpp);
DisplaySurface *qemu_create_displaysurface(int width, int height);
@@ -322,7 +320,6 @@ void qemu_console_resize(QemuConsole *con, int width, int
height);
void qemu_console_copy(QemuConsole *con, int src_x, int src_y,
int dst_x, int dst_y, int w, int h);
DisplaySurface *qemu_console_surface(QemuConsole *con);
-DisplayState *qemu_console_displaystate(QemuConsole *console);
/* sdl.c */
void sdl_display_init(DisplayState *ds, int full_screen, int no_frame);
diff --git a/ui/console.c b/ui/console.c
index 87574a7..87af6b5 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -2005,18 +2005,6 @@ DisplaySurface *qemu_console_surface(QemuConsole
*console)
return console->surface;
}
-DisplayState *qemu_console_displaystate(QemuConsole *console)
-{
-return console->ds;
-}
-
-PixelFormat qemu_different_endianness_pixelformat(int bpp)
-{
-pixman_format_code_t fmt = qemu_default_pixman_format(bpp, false);
-PixelFormat pf = qemu_pixelformat_from_pixman(fmt);
-return pf;
-}
-
PixelFormat qemu_default_pixelformat(int bpp)
{
pixman_format_code_t fmt = qemu_default_pixman_format(bpp, true);
diff --git a/ui/d3des.c b/ui/d3des.c
index 60c840e..5bc99b8 100644
--- a/ui/d3des.c
+++ b/ui/d3des.c
@@ -121,15 +121,6 @@ static void cookey(register unsigned long *raw1)
return;
}
-void cpkey(register unsigned long *into)
-{
- register unsigned long *from, *endp;
-
- from = KnL, endp = &KnL[32];
- while( from < endp ) *into++ = *from++;
- return;
- }
-
void usekey(register unsigned long *from)
{
register unsigned long *to, *endp;
diff --git a/ui/d3des.h b/ui/d3des.h
index 70cb6b5..773667e 100644
--- a/ui/d3des.h
+++ b/ui/d3des.h
@@ -36,12 +36,6 @@ void usekey(unsigned long *);
* Loads the internal key register with the data in cookedkey.
*/
-void cpkey(unsigned long *);
-/*cookedkey[32]
- * Copies the contents of the internal key register into the storage
- * located at &cookedkey[0].
- */
-
void des(unsigned char *, unsigned char *);
/* from[8] to[8]
* Encrypts/Decrypts (according to the key currently loaded in the
diff --git a/ui/input-legacy.c b/ui/input-legacy.c
index a698a34..2d4ca19 100644
--- a/ui/input-legacy.c
+++ b/ui/input-legacy.c
@@ -143,12 +143,6 @@ QEMUPutKbdEntry
*qemu_add_kbd_event_handler(QEMUPutKBDEvent *func, void *opaque)
return entry;
}
-void qemu_remove_kbd_event_handler(QEMUPutKbdEntry *entry)
-{
-qemu_input_handler_unregister(entry->s);
-g_free(entry);
-}
-
static void legacy_mouse_event(DeviceState *dev, QemuConsole *src,
InputEvent *evt)
{
--
1.7.1
[Qemu-devel] [PATCH 3/3] xen: Remove xen_cmos_set_s3_resume()
The function is not used anymore, and thus can be deleted.
Signed-off-by: Thomas Huth
Acked-by: Stefano Stabellini
---
include/hw/xen/xen.h |1 -
xen-hvm-stub.c |4
2 files changed, 0 insertions(+), 5 deletions(-)
diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
index b0ed04c..4356af4 100644
--- a/include/hw/xen/xen.h
+++ b/include/hw/xen/xen.h
@@ -32,7 +32,6 @@ int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num);
void xen_piix3_set_irq(void *opaque, int irq_num, int level);
void xen_piix_pci_write_config_client(uint32_t address, uint32_t val, int len);
void xen_hvm_inject_msi(uint64_t addr, uint32_t data);
-void xen_cmos_set_s3_resume(void *opaque, int irq, int level);
qemu_irq *xen_interrupt_controller_init(void);
diff --git a/xen-hvm-stub.c b/xen-hvm-stub.c
index 2d98696..46867d8 100644
--- a/xen-hvm-stub.c
+++ b/xen-hvm-stub.c
@@ -30,10 +30,6 @@ void xen_hvm_inject_msi(uint64_t addr, uint32_t data)
{
}
-void xen_cmos_set_s3_resume(void *opaque, int irq, int level)
-{
-}
-
void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr)
{
}
--
1.7.1
Re: [Qemu-devel] [PATCH RFC] coverity: Improve model for GLib memory allocation
On Thu, 22 Jan 2015 15:55:27 +0100 Markus Armbruster wrote: ... > > == Triaged / Bug == > > These are worrying. Something wrong with my new model? > > hw/s390x/s390-pci-bus.c:195: leaked_storage: Variable "sei_cont" going out of > scope leaks the storage it points to. Did you already include the fix for the sei_count leak before you ran the test? (http://lists.nongnu.org/archive/html/qemu-devel/2015-01/msg02348.html) Thomas
Re: [Qemu-devel] makefile help: giving QEMU an icon
On Fri, 23 Jan 2015 15:48:31 -0500 Programmingkid wrote: > I'm trying to make QEMU have an icon instead of the standard gray box icon it > is given on Mac OS X. I figured out where to put the code in the makefile, > but this location isn't useful. git is trained not to use it. The location is > ./ppc-softmmu/makefile. My question is where do I put my icon setting code? > > It currently works in ./ppc-softmmu/makefile after this line: all: $(PROGS) > stap Well, that file is normally just a symlink, generated by the configure script. So you might want to have a look at the destination of the link instead, the Makefile.target file in the main directory of the sources. Thomas
[Qemu-devel] [PATCH] qemu-log: Correct help text of 'log cpu_reset'
The logging of the CPU state during reset is done for all architectures
nowadays (see cpu_common_reset() in qom/cpu.c), so the "x86 only" text
does not apply here anymore.
Signed-off-by: Thomas Huth
---
qemu-log.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/qemu-log.c b/qemu-log.c
index 05b5493..13f3813 100644
--- a/qemu-log.c
+++ b/qemu-log.c
@@ -111,7 +111,7 @@ const QEMULogItem qemu_log_items[] = {
{ CPU_LOG_PCALL, "pcall",
"x86 only: show protected mode far calls/returns/exceptions" },
{ CPU_LOG_RESET, "cpu_reset",
- "x86 only: show CPU state before CPU resets" },
+ "show CPU state before CPU resets" },
{ CPU_LOG_IOPORT, "ioport",
"show all i/o ports accesses" },
{ LOG_UNIMP, "unimp",
--
1.7.1
Re: [Qemu-devel] [PATCH 2/6] rtl8139: g_malloc() can't fail, bury dead error handling
On Tue, 27 Jan 2015 17:38:27 +0100
Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster
> ---
> hw/net/rtl8139.c | 14 --
> 1 file changed, 14 deletions(-)
>
> diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
> index 6fa9e0a..b55e438 100644
> --- a/hw/net/rtl8139.c
> +++ b/hw/net/rtl8139.c
> @@ -2075,20 +2075,6 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
> "length to %d\n", txsize);
> }
>
> -if (!s->cplus_txbuffer)
> -{
> -/* out of memory */
> -
> -DPRINTF("+++ C+ mode transmiter failed to reallocate %d bytes\n",
> -s->cplus_txbuffer_len);
> -
> -/* update tally counter */
> -++s->tally_counters.TxERR;
> -++s->tally_counters.TxAbt;
> -
> -return 0;
> -}
> -
> /* append more data to the packet */
>
> DPRINTF("+++ C+ mode transmit reading %d bytes from host memory at "
Wouldn't it be better to use g_try_malloc() here instead? If the code
can handle OOM conditions, I think it's better to continue with a lost
packet here than to shut down QEMU the hard way.
(Also looking at the history of that file, the code originally used
qemu_malloc() which could fail - but instead of being replaced by
g_try_malloc(), it got replaced with g_malloc() instead which was
maybe the wrong decision).
Thomas
Re: [Qemu-devel] [PATCH 5/6] vnc: g_realloc() can't fail, bury dead error handling
On Tue, 27 Jan 2015 17:38:30 +0100
Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster
> ---
> ui/vnc.c | 4
> 1 file changed, 4 deletions(-)
>
> diff --git a/ui/vnc.c b/ui/vnc.c
> index a742c90..02552ee 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -659,10 +659,6 @@ void buffer_reserve(Buffer *buffer, size_t len)
> if ((buffer->capacity - buffer->offset) < len) {
> buffer->capacity += (len + 1024);
> buffer->buffer = g_realloc(buffer->buffer, buffer->capacity);
> -if (buffer->buffer == NULL) {
> -fprintf(stderr, "vnc: out of memory\n");
> -exit(1);
> -}
> }
> }
>
Reviewed-by: Thomas Huth
Re: [Qemu-devel] [PATCH 3/6] kvm: g_malloc() can't fail, bury dead error handling
On Tue, 27 Jan 2015 17:38:28 +0100
Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster
> ---
> kvm-all.c | 4
> 1 file changed, 4 deletions(-)
>
> diff --git a/kvm-all.c b/kvm-all.c
> index 2f21a4e..05a79c2 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -2070,10 +2070,6 @@ int kvm_insert_breakpoint(CPUState *cpu, target_ulong
> addr,
> }
>
> bp = g_malloc(sizeof(struct kvm_sw_breakpoint));
> -if (!bp) {
> -return -ENOMEM;
> -}
> -
> bp->pc = addr;
> bp->use_count = 1;
> err = kvm_arch_insert_sw_breakpoint(cpu, bp);
I think I'd also use g_try_malloc() here instead. Looks like an error
gets reported to GDB when this function returns with an error code, so
returning -ENOMEM should be ok here, shouldn't it?
Thomas
Re: [Qemu-devel] [PATCH RFC v6 05/20] virtio: support more feature bits
Hi, On Thu, 29 Jan 2015 11:11:32 +1100 David Gibson wrote: > On Wed, Jan 28, 2015 at 04:59:45PM +0100, Cornelia Huck wrote: > > On Thu, 22 Jan 2015 12:43:43 +1100 > > David Gibson wrote: > > > > > On Thu, Dec 11, 2014 at 02:25:07PM +0100, Cornelia Huck wrote: > > > > With virtio-1, we support more than 32 feature bits. Let's extend both > > > > host and guest features to 64, which should suffice for a while. > > > > > > > > vhost and migration have been ignored for now. > > > > > > [snip] > > > > > > > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h > > > > index f6c0379..08141c7 100644 > > > > --- a/include/hw/virtio/virtio.h > > > > +++ b/include/hw/virtio/virtio.h > > > > @@ -55,6 +55,12 @@ > > > > /* A guest should never accept this. It implies negotiation is > > > > broken. */ > > > > #define VIRTIO_F_BAD_FEATURE 30 > > > > > > > > +/* v1.0 compliant. */ > > > > +#define VIRTIO_F_VERSION_1 32 > > > > > > This is already in the kernel header, isn't it? > > > > > Yes. But nearly all files include this header but not the kernel > > header. > > Can't you change that? Or this file include the kernel header? AFAIK non-KVM code should never try to include one of the Linux headers to avoid breaking on non-Linux platforms (for example is not available on OS X, see http://patchwork.ozlabs.org/patch/424655/ ). So it's a little bit ugly to define these things twice, but it seems the only way to stay portable. Thomas signature.asc Description: PGP signature
[Qemu-devel] [PATCH RFC 0/1] KVM: ioctl for reading/writing guest memory
tl;dr: This patch adds a new ioctl to KVM on s390x for reading and writing from/to virtual guest memory, to take account of the so-called IPTE-lock on s390x (a locking mechanism for the host to walk MMU tables of the guest). Long story: Certain instruction interception handlers in QEMU have to access the memory of the guest, either to retrieve additional paramaters/data or to supply results to the guest. On s390x, some of them (e.g. MSCH, SSCH, STSCH, ...) are specified to use logical (i.e. virtual) addresses in memory, i.e. the addresses are subject to MMU translation. The current handlers in target-s390x/ioinst.c just work "by accident" since the Linux kernel on s390x uses a 1:1 MMU mapping for kernel memory, but for correct behaviour we have to do a MMU page table walk in these handlers first. Now on s390x, there's another specialty for the case the host has to walk the MMU tables of the guest: While doing the page table walk (or while accessing the memory of the guest in bigger, non-atomic chunks on multiple pages), there is a small chance that another CPU might zap or change the MMU mappings inbetween, so in that case an unexpected/undefined behaviour might occur. To avoid such problems, the SIE facility features a locking mechanism, the so called IPTE-lock, which prevents other virtual CPUs from issuing the IPTE (invalidate page table entry) or similar instructions. When the lock is being held, these other instructions are intercepted, so that the execution of the instructions can be delayed until the page table walk / memory operation finished on the locking CPU. The kernel part of KVM on s390x already uses this locking mechanism for the interception handlers in the kernel (e.g. during the read_guest() and write_guest() functions). For proper MMU page table walk support in QEMU, the IPTE-lock has now somehow to be provided to the userspace, too. However, providing this lock directly to the userspace would be quite ugly, since we then need to deal with a lot of cumbersome conditions (how should the kernel behave if userspace takes the lock for too long or forgets to free it again etc.). Additionally, there is also another specialty of s390x pending - proper handling of the so-called storage keys when accessing the guest memory - which is also done best in the kernel space instead of user space (I can elaborate more on that topic on request). So I decided to introduce a simple ioctl for reading and writing from/to guest memory instead of exporting the lock itself to userspace. The userspace (QEMU) then can simply call this ioctl when it wants to read or write from/to virtual guest memory. Then kernel then takes the IPTE-lock, walks the MMU table of the guest to find out the physical address that corresponds to the virtual address, copies the requested amount of bytes from the userspace buffer to guest memory or the other way round, and finally frees the IPTE-lock again. Does that sound like a viable solution (IMHO it does ;-))? Or should I maybe try to pursue another approach? Thomas Huth (1): KVM: s390: Add MEMOP ioctls for reading/writing guest memory Documentation/virtual/kvm/api.txt | 44 + arch/s390/kvm/gaccess.c | 22 + arch/s390/kvm/gaccess.h |2 + arch/s390/kvm/kvm-s390.c | 63 + include/uapi/linux/kvm.h | 21 5 files changed, 152 insertions(+), 0 deletions(-)
[Qemu-devel] [PATCH RFC 1/1] KVM: s390: Add MEMOP ioctl for reading/writing guest memory
On s390, we've got to make sure to hold the IPTE lock while accessing
virtual memory. So let's add an ioctl for reading and writing virtual
memory to provide this feature for userspace, too.
Signed-off-by: Thomas Huth
Reviewed-by: Dominik Dingel
Reviewed-by: David Hildenbrand
---
Documentation/virtual/kvm/api.txt | 44 +
arch/s390/kvm/gaccess.c | 22 +
arch/s390/kvm/gaccess.h |2 +
arch/s390/kvm/kvm-s390.c | 63 +
include/uapi/linux/kvm.h | 21
5 files changed, 152 insertions(+), 0 deletions(-)
diff --git a/Documentation/virtual/kvm/api.txt
b/Documentation/virtual/kvm/api.txt
index b112efc..bf44b53 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -2716,6 +2716,50 @@ The fields in each entry are defined as follows:
eax, ebx, ecx, edx: the values returned by the cpuid instruction for
this function/index combination
+4.89 KVM_GUEST_MEM_OP
+
+Capability: KVM_CAP_MEM_OP
+Architectures: s390
+Type: vcpu ioctl
+Parameters: struct kvm_guest_mem_op (in)
+Returns: = 0 on success,
+ < 0 on generic error (e.g. -EFAULT or -ENOMEM),
+ > 0 if an exception occurred while walking the page tables
+
+Read or write data from/to the virtual memory of a VPCU.
+
+Parameters are specified via the following structure:
+
+struct kvm_guest_mem_op {
+ __u64 gaddr;/* the guest address */
+ __u64 flags;/* arch specific flags */
+ __u32 size; /* amount of bytes */
+ __u32 op; /* type of operation */
+ __u64 buf; /* buffer in userspace */
+ __u8 reserved[32]; /* should be set to 0 */
+};
+
+The type of operation is specified in the "op" field, either KVM_MEMOP_VIRTREAD
+for reading from memory, KVM_MEMOP_VIRTWRITE for writing to memory, or
+KVM_MEMOP_CHECKVIRTREAD or KVM_MEMOP_CHECKVIRTWRITE to check whether the
+corresponding memory access would create an access exception (without
+changing the data in the memory at the destination). In case an access
+exception occurred while walking the MMU tables of the guest, the ioctl
+returns a positive error number to indicate the type of exception. The
+exception is raised directly at the corresponding VCPU if the bit
+KVM_MEMOP_F_INJECT_EXC is set in the "flags" field.
+
+The logical (virtual) start address of the memory region has to be specified
+in the "gaddr" field, and the length of the region in the "size" field.
+"buf" is the buffer supplied by the userspace application where the read data
+should be written to for KVM_MEMOP_VIRTREAD, or where the data that should
+be written is stored for a KVM_MEMOP_VIRTWRITE. "buf" can be NULL for both
+CHECK operations.
+
+The "reserved" field is meant for future extensions. It must currently be
+set to 0.
+
+
5. The kvm_run structure
diff --git a/arch/s390/kvm/gaccess.c b/arch/s390/kvm/gaccess.c
index 8a1be90..d912362 100644
--- a/arch/s390/kvm/gaccess.c
+++ b/arch/s390/kvm/gaccess.c
@@ -697,6 +697,28 @@ int guest_translate_address(struct kvm_vcpu *vcpu,
unsigned long gva,
}
/**
+ * check_gva_range - test a range of guest virtual addresses for accessibility
+ */
+int check_gva_range(struct kvm_vcpu *vcpu, unsigned long gva,
+ unsigned long length, int is_write)
+{
+ unsigned long gpa;
+ unsigned long currlen;
+ int rc = 0;
+
+ ipte_lock(vcpu);
+ while (length > 0 && !rc) {
+ currlen = min(length, PAGE_SIZE - (gva % PAGE_SIZE));
+ rc = guest_translate_address(vcpu, gva, &gpa, is_write);
+ gva += currlen;
+ length -= currlen;
+ }
+ ipte_unlock(vcpu);
+
+ return rc;
+}
+
+/**
* kvm_s390_check_low_addr_protection - check for low-address protection
* @ga: Guest address
*
diff --git a/arch/s390/kvm/gaccess.h b/arch/s390/kvm/gaccess.h
index 0149cf1..268beb7 100644
--- a/arch/s390/kvm/gaccess.h
+++ b/arch/s390/kvm/gaccess.h
@@ -157,6 +157,8 @@ int read_guest_lc(struct kvm_vcpu *vcpu, unsigned long gra,
void *data,
int guest_translate_address(struct kvm_vcpu *vcpu, unsigned long gva,
unsigned long *gpa, int write);
+int check_gva_range(struct kvm_vcpu *vcpu, unsigned long gva,
+ unsigned long length, int is_write);
int access_guest(struct kvm_vcpu *vcpu, unsigned long ga, void *data,
unsigned long len, int write);
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index b2371c0..c80a640 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -168,6 +168,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
case KVM_CAP_VM_ATTRIBUTES:
case KVM_
Re: [Qemu-devel] [PATCH RFC 1/1] KVM: s390: Add MEMOP ioctl for reading/writing guest memory
On Tue, 03 Feb 2015 14:04:43 +0100
Paolo Bonzini wrote:
> On 03/02/2015 13:11, Thomas Huth wrote:
> > On s390, we've got to make sure to hold the IPTE lock while accessing
> > virtual memory. So let's add an ioctl for reading and writing virtual
> > memory to provide this feature for userspace, too.
> >
> > Signed-off-by: Thomas Huth
> > Reviewed-by: Dominik Dingel
> > Reviewed-by: David Hildenbrand
> > ---
> > Documentation/virtual/kvm/api.txt | 44 +
> > arch/s390/kvm/gaccess.c | 22 +
> > arch/s390/kvm/gaccess.h |2 +
> > arch/s390/kvm/kvm-s390.c | 63
> > +
> > include/uapi/linux/kvm.h | 21
> > 5 files changed, 152 insertions(+), 0 deletions(-)
> >
> > diff --git a/Documentation/virtual/kvm/api.txt
> > b/Documentation/virtual/kvm/api.txt
> > index b112efc..bf44b53 100644
> > --- a/Documentation/virtual/kvm/api.txt
> > +++ b/Documentation/virtual/kvm/api.txt
> > @@ -2716,6 +2716,50 @@ The fields in each entry are defined as follows:
> > eax, ebx, ecx, edx: the values returned by the cpuid instruction for
> > this function/index combination
> >
> > +4.89 KVM_GUEST_MEM_OP
> > +
> > +Capability: KVM_CAP_MEM_OP
>
> Put "virtual" somewhere in the ioctl name and capability?
Actually, I'd prefer to keep the "virtual" in the defines for the type
of operation below: When it comes to s390 storage keys, we likely might
need some calls for reading and writing to physical memory, too. Then
we could simply extend this ioctl instead of inventing a new one.
> > +Architectures: s390
> > +Type: vcpu ioctl
> > +Parameters: struct kvm_guest_mem_op (in)
> > +Returns: = 0 on success,
> > + < 0 on generic error (e.g. -EFAULT or -ENOMEM),
> > + > 0 if an exception occurred while walking the page tables
> > +
> > +Read or write data from/to the virtual memory of a VPCU.
> > +
> > +Parameters are specified via the following structure:
> > +
> > +struct kvm_guest_mem_op {
> > + __u64 gaddr;/* the guest address */
> > + __u64 flags;/* arch specific flags */
> > + __u32 size; /* amount of bytes */
> > + __u32 op; /* type of operation */
> > + __u64 buf; /* buffer in userspace */
> > + __u8 reserved[32]; /* should be set to 0 */
> > +};
> > +
> > +The type of operation is specified in the "op" field, either
> > KVM_MEMOP_VIRTREAD
> > +for reading from memory, KVM_MEMOP_VIRTWRITE for writing to memory, or
> > +KVM_MEMOP_CHECKVIRTREAD or KVM_MEMOP_CHECKVIRTWRITE to check whether the
>
> Better:
>
> #define KVM_MEMOP_READ 0
> #define KVM_MEMOP_WRITE 1
>
> and in the flags field:
>
> #define KVM_MEMOP_F_CHECK_ONLY (1 << 1)
Ok, a flag for the check operations is fine for me, too.
...
> > +The logical (virtual) start address of the memory region has to be
> > specified
> > +in the "gaddr" field, and the length of the region in the "size" field.
> > +"buf" is the buffer supplied by the userspace application where the read
> > data
> > +should be written to for KVM_MEMOP_VIRTREAD, or where the data that should
> > +be written is stored for a KVM_MEMOP_VIRTWRITE. "buf" can be NULL for both
> > +CHECK operations.
>
> "buf" is unused and can be NULL for both CHECK operations.
>
> > +The "reserved" field is meant for future extensions. It must currently be
> > +set to 0.
>
> Not really true, as you don't check it. So "It is not used by KVM with
> the currently defined set of flags" is a better explanation.
ok ... and maybe add "should be set to zero" ?
> Paolo
Thanks for the review!
Thomas
Re: [Qemu-devel] [PATCH RFC 1/1] KVM: s390: Add MEMOP ioctl for reading/writing guest memory
On Tue, 03 Feb 2015 16:22:32 +0100 Paolo Bonzini wrote: > > > On 03/02/2015 16:16, Thomas Huth wrote: > > Actually, I'd prefer to keep the "virtual" in the defines for the type > > of operation below: When it comes to s390 storage keys, we likely might > > need some calls for reading and writing to physical memory, too. Then > > we could simply extend this ioctl instead of inventing a new one. > > Can you explain why it is necessary to read/write physical addresses > from user space? In the case of QEMU, I'm worried that you would have > to invent your own memory read/write APIs that are different from > everything else. > > On real s390 zPCI, does bus-master DMA update storage keys? Ah, I was not thinking about bus-mastering/DMA here: AFAIK there are some CPU instructions that access a parameter block in physical memory, for example the SCLP instruction (see hw/s390x/sclp.c) - it's already doing a cpu_physical_memory_read and ..._write for the parameters. However, I haven't checked yet whether it is also supposed to touch the storage keys, so if not, we also might be fine without the ioctls for reading/writing to physical memory. > >> Not really true, as you don't check it. So "It is not used by KVM with > >> the currently defined set of flags" is a better explanation. > > > > ok ... and maybe add "should be set to zero" ? > > If you don't check it, it is misleading to document this. True... so I'll omit that. Thomas
Re: [Qemu-devel] [PATCH RFC 1/1] KVM: s390: Add MEMOP ioctl for reading/writing guest memory
On Wed, 04 Feb 2015 09:26:11 +0100 Christian Borntraeger wrote: > Am 03.02.2015 um 16:22 schrieb Paolo Bonzini: > > > > > > On 03/02/2015 16:16, Thomas Huth wrote: > >> Actually, I'd prefer to keep the "virtual" in the defines for the type > >> of operation below: When it comes to s390 storage keys, we likely might > >> need some calls for reading and writing to physical memory, too. Then > >> we could simply extend this ioctl instead of inventing a new one. > > Rereading that. Shall we replace "virtual" with "logical"? That is what is > used architecturally when we mean "do whatever is appropriate right now" > That can boil down to virtual via DAT, virtual via access register mode, > real if DAT is off... and if fact your kernel implementation does that. True, but so far I tried to avoid to include s390-only wording into this ioctl in case other architectures might need such a functionality later, too (then this ioctl could be simply used there, too). OTOH, maybe the memory model on s390 is just too special to try to keep this ioctl generic ... but then I guess I also should rename the define KVM_GUEST_MEM_OP into KVM_S390_GUEST_MEM_OP, for example? Thomas
[Qemu-devel] [PATCH v2] hw/usb: Include USB files only if necessary
Boards that do not include an USB controller should not provide USB devices. However, when running "qemu-system-s390x -device help" for example, there's still a usb-hub, usb-kbd, usb-mouse and usb-tablet in the list of "supported" devices. Let's fix that by compiling and linking the USB files only if it is really necessary. Signed-off-by: Thomas Huth --- v2: Everything should now also compile fine again when configure has been run with the --enable-usb-redir and/or --enable-libusb option. --- default-configs/arm-softmmu.mak |1 + default-configs/usb.mak |1 + hw/usb/Makefile.objs|8 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak index 87d4e34..a767e4b 100644 --- a/default-configs/arm-softmmu.mak +++ b/default-configs/arm-softmmu.mak @@ -32,6 +32,7 @@ CONFIG_DS1338=y CONFIG_PFLASH_CFI01=y CONFIG_PFLASH_CFI02=y CONFIG_MICRODRIVE=y +CONFIG_USB=y CONFIG_USB_MUSB=y CONFIG_USB_EHCI_SYSBUS=y CONFIG_PLATFORM_BUS=y diff --git a/default-configs/usb.mak b/default-configs/usb.mak index 73d8489..f4b8568 100644 --- a/default-configs/usb.mak +++ b/default-configs/usb.mak @@ -1,3 +1,4 @@ +CONFIG_USB=y CONFIG_USB_TABLET_WACOM=y CONFIG_USB_STORAGE_BOT=y CONFIG_USB_STORAGE_UAS=y diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs index 0ccd477..7443e38 100644 --- a/hw/usb/Makefile.objs +++ b/hw/usb/Makefile.objs @@ -1,6 +1,6 @@ # usb subsystem core -common-obj-y += core.o combined-packet.o bus.o desc.o desc-msos.o -common-obj-y += libhw.o +common-obj-y += core.o combined-packet.o bus.o libhw.o +common-obj-$(CONFIG_USB) += desc.o desc-msos.o # usb host adapters common-obj-$(CONFIG_USB_UHCI) += hcd-uhci.o @@ -11,8 +11,8 @@ common-obj-$(CONFIG_USB_XHCI) += hcd-xhci.o common-obj-$(CONFIG_USB_MUSB) += hcd-musb.o # emulated usb devices -common-obj-y += dev-hub.o -common-obj-y += dev-hid.o +common-obj-$(CONFIG_USB) += dev-hub.o +common-obj-$(CONFIG_USB) += dev-hid.o common-obj-$(CONFIG_USB_TABLET_WACOM) += dev-wacom.o common-obj-$(CONFIG_USB_STORAGE_BOT) += dev-storage.o common-obj-$(CONFIG_USB_STORAGE_UAS) += dev-uas.o -- 1.7.1
[Qemu-devel] [RFC PATCH] libcacard: Fix bug detected with 'smatch'
'smatch' complains about two bugs and one style issue in card_7816.c:
libcacard/card_7816.c:273 vcard_apdu_set_length() warn: should this be a
bitwise op?
libcacard/card_7816.c:295 vcard_apdu_set_length() warn: should this be a
bitwise op?
libcacard/card_7816.c:661 vcard7816_vm_process_apdu() warn: inconsistent
indenting
... and indeed, the code seems to be wrong here. Let's fix this
by using a bitwise OR instead of logical OR and by indenting
the code with the right level.
Signed-off-by: Thomas Huth
---
Please note that this is compile-tested only. I don't have a clue
about that libcacard stuff, so if you feel confident in this area,
please have a look whether this change really makes sense.
---
libcacard/card_7816.c |6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/libcacard/card_7816.c b/libcacard/card_7816.c
index 814fa16..0f7a006 100644
--- a/libcacard/card_7816.c
+++ b/libcacard/card_7816.c
@@ -270,7 +270,7 @@ vcard_apdu_set_length(VCardAPDU *apdu)
}
/* calculate the first extended value. Could be either Le or Lc */
Le = (apdu->a_header->ah_body[0] << 8)
- || apdu->a_header->ah_body[1];
+| apdu->a_header->ah_body[1];
if (L == 3) {
/* 2E extended, return data only */
/* zero maps to 65536 */
@@ -292,7 +292,7 @@ vcard_apdu_set_length(VCardAPDU *apdu)
if (L == Le+5) {
/* 4E extended, parameters and return data */
Le = (apdu->a_data[apdu->a_len-2] << 8)
- || apdu->a_data[apdu->a_len-1];
+| apdu->a_data[apdu->a_len-1];
apdu->a_Le = Le ? Le : 65536;
return VCARD7816_STATUS_SUCCESS;
}
@@ -657,7 +657,7 @@ vcard7816_vm_process_apdu(VCard *card, VCardAPDU *apdu,
}
}
} else {
-status = vcard_emul_login(card, apdu->a_body, apdu->a_Lc);
+status = vcard_emul_login(card, apdu->a_body, apdu->a_Lc);
*response = vcard_make_response(status);
}
}
--
1.7.1
Re: [Qemu-devel] [PATCH v2 3/7] pci: Remove unused function ich9_d2pbr_init()
On Wed, 18 Mar 2015 14:20:41 +0100 "Michael S. Tsirkin" wrote: > On Sat, Mar 14, 2015 at 07:19:30AM +0100, Thomas Huth wrote: > > The function ich9_d2pbr_init() is completely unused and > > thus can be deleted. > > > > Signed-off-by: Thomas Huth > > Cc: Michael S. Tsirkin > > Reviewed-by: Michael S. Tsirkin > > I assume all this will go in for 2.4 through the trivial tree? The patch that removes unused functions from the migration code has already been picked up in the migration tree ... so I also don't mind whether you want to take this through the pci tree or not. Just if the patches are not picked up through other trees, they should finally go through trivial, I think. Thomas
Re: [Qemu-devel] [PATCH for-2.3 3/4] s390x: do not include ram_addr.h
Am Thu, 26 Mar 2015 16:36:01 +0100 schrieb Cornelia Huck : > From: Paolo Bonzini > > ram_addr.h is an internal interface and it is not needed anyway by > hw/s390x/ipl.c. > > Cc: Christian Borntraeger > Signed-off-by: Paolo Bonzini > Message-Id: <1427295389-5054-1-git-send-email-pbonz...@redhat.com> > Signed-off-by: Christian Borntraeger > Signed-off-by: Cornelia Huck > --- > hw/s390x/ipl.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c > index 54d0835..5c86613 100644 > --- a/hw/s390x/ipl.c > +++ b/hw/s390x/ipl.c > @@ -14,7 +14,6 @@ > #include "sysemu/sysemu.h" > #include "cpu.h" > #include "elf.h" > -#include "exec/ram_addr.h" > #include "hw/loader.h" > #include "hw/sysbus.h" > #include "hw/s390x/virtio-ccw.h" Uh, IIRC I temporarily played around with qemu_get_ram_ptr() when I recently improved the BIOS loading in this file - and later forgot to remove that include file again - sorry! So of course, it should not be here. Thanks for cleaning it up! Reviewed-by: Thomas Huth
Re: [Qemu-devel] [PATCH] Makefile: Remove config.status and common.env during 'make distclean'
On Mon, 15 Dec 2014 11:19:46 +0100 Thomas Huth wrote: > config.status and tests/qemu-iotests/common.env are generated files > that should be deleted during 'make distclean'. > > Signed-off-by: Thomas Huth > --- > Makefile |4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/Makefile b/Makefile > index f505202..6817c6f 100644 > --- a/Makefile > +++ b/Makefile > @@ -313,8 +313,8 @@ qemu-%.tar.bz2: > > distclean: clean > rm -f config-host.mak config-host.h* config-host.ld $(DOCS) > qemu-options.texi qemu-img-cmds.texi qemu-monitor.texi > - rm -f config-all-devices.mak config-all-disas.mak > - rm -f po/*.mo > + rm -f config-all-devices.mak config-all-disas.mak config.status > + rm -f po/*.mo tests/qemu-iotests/common.env > rm -f roms/seabios/config.mak roms/vgabios/config.mak > rm -f qemu-doc.info qemu-doc.aux qemu-doc.cp qemu-doc.cps qemu-doc.dvi > rm -f qemu-doc.fn qemu-doc.fns qemu-doc.info qemu-doc.ky qemu-doc.kys ping?
Re: [Qemu-devel] [PATCH] tcg: Add doxygen documentation to the tcg frontend
On Wed, 14 Jan 2015 16:36:26 +0100 Lluís Vilanova wrote: ... > > Also, AFAIR it was decided to use gtk-doc instead of doxygen. If there's a consensus about which source code documentation style should be used for QEMU, could you (or somebody else) maybe add an appropriate paragraph to the CODING_STYLE file to avoid future confusion? Thomas
Re: [Qemu-devel] [PATCH] s390x/pci: fix 2 bugs found by coverity
On Wed, 14 Jan 2015 16:20:47 +0100
Frank Blaschka wrote:
> Signed-off-by: Frank Blaschka
> ---
> hw/s390x/s390-pci-bus.c | 1 +
> hw/s390x/s390-pci-inst.c | 2 +-
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> index 1201b8d..546dcf1 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -192,6 +192,7 @@ static void s390_pci_generate_event(uint8_t cc, uint16_t
> pec, uint32_t fh,
> object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
>
> if (!s) {
> +g_free(sei_cont);
> return;
> }
>
> diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
> index 5ea13e5..5596679 100644
> --- a/hw/s390x/s390-pci-inst.c
> +++ b/hw/s390x/s390-pci-inst.c
> @@ -787,7 +787,7 @@ int stpcifc_service_call(S390CPU *cpu, uint8_t r1,
> uint64_t fiba)
> data = (pbdev->isc << 28) | (pbdev->noi << 16) |
> (pbdev->routes.adapter.ind_offset << 8) | (pbdev->sum << 7) |
> pbdev->routes.adapter.summary_offset;
> -stw_p(&fib.data, data);
> +stl_p(&fib.data, data);
>
> if (pbdev->fh >> ENABLE_BIT_OFFSET) {
> fib.fc |= 0x80;
Looks right.
Reviewed-by: Thomas Huth
Re: [Qemu-devel] [PATCH 5/6] linux-user/main.c: Mark end_exclusive() as possibly unused
On Thu, 8 Jan 2015 12:19:47 +
Peter Maydell wrote:
> The function end_exclusive() isn't used on all targets; mark it as
> such to avoid a clang warning.
>
> Signed-off-by: Peter Maydell
> ---
> linux-user/main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/linux-user/main.c b/linux-user/main.c
> index bbd1cfd..0fda51c 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -169,7 +169,7 @@ static inline void start_exclusive(void)
> }
>
> /* Finish an exclusive operation. */
> -static inline void end_exclusive(void)
> +static inline void __attribute__((unused)) end_exclusive(void)
> {
> pending_cpus = 0;
> pthread_cond_broadcast(&exclusive_resume);
IMHO it might be better to add a proper #ifdef guard around that
function. Consider that the calls to end_exclusive() might get removed
completely one day, then you won't get a compiler warning about the
unused function anymore if you used the attribute__((unused)) way.
Thomas
