Re: [Xen-devel] [PATCH v19 12/13] x86/hvm: Remove redundant save functions
>>> On 10.09.18 at 15:54, wrote:
> On Mon, 2018-09-10 at 07:42 -0600, Jan Beulich wrote:
>> > > > On 10.09.18 at 15:33, wrote:
>> >
>> > On Mon, 2018-09-10 at 15:36 +0300, Alexandru Isaila wrote:
>> > > This patch removes the redundant save functions and renames the
>> > > save_one* to save. It then changes the domain param to vcpu in
>> > > the
>> > > save funcs and adapts print messages in order to match the format
>> > > of
>> > > the
>> > > other save related messages.
>> > >
>> > > Signed-off-by: Alexandru Isaila
>> > >
>> > > ---
>> > > Changes since V18:
>> > > - Add const struct domain to rtc_save and hpet_save
>> > > - Latched the vCPU into a local variable in hvm_save_one()
>> > > - Add HVMSR_PER_VCPU kind check to the bounds if.
>> > > ---
>> > > xen/arch/x86/cpu/mcheck/vmce.c | 18 +---
>> > > xen/arch/x86/emul-i8254.c | 5 ++-
>> > > xen/arch/x86/hvm/hpet.c| 7 ++--
>> > > xen/arch/x86/hvm/hvm.c | 75 +++-
>> > >
>> > > --
>> > > xen/arch/x86/hvm/irq.c | 15 ---
>> > > xen/arch/x86/hvm/mtrr.c| 22 ++
>> > > xen/arch/x86/hvm/pmtimer.c | 5 ++-
>> > > xen/arch/x86/hvm/rtc.c | 5 ++-
>> > > xen/arch/x86/hvm/save.c| 28 +++--
>> > > xen/arch/x86/hvm/vioapic.c | 5 ++-
>> > > xen/arch/x86/hvm/viridian.c| 23 ++-
>> > > xen/arch/x86/hvm/vlapic.c | 38 ++---
>> > > xen/arch/x86/hvm/vpic.c| 5 ++-
>> > > xen/include/asm-x86/hvm/save.h | 8 +---
>> > > 14 files changed, 63 insertions(+), 196 deletions(-)
>> > >
>> > > @@ -141,6 +138,8 @@ int hvm_save_one(struct domain *d, unsigned
>> > > int
>> > > typecode, unsigned int instance,
>> > > int rv;
>> > > hvm_domain_context_t ctxt = { };
>> > > const struct hvm_save_descriptor *desc;
>> > > +struct vcpu *v = (hvm_sr_handlers[typecode].kind ==
>> > > HVMSR_PER_VCPU) ?
>> > > + d->vcpu[instance] : d->vcpu[0];
>> > >
>> >
>> > Sorry for the inconvenience but I've just realized that this has to
>> > be
>> > initialize after the bounds check. I will have this in mine
>>
>> Also to eliminate redundancy I'd prefer if you moved the conditional
>> expression inside the square brackets.
>>
> Are these changes worth waiting 24h?
That's up to you in this case, I'd say.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v19 12/13] x86/hvm: Remove redundant save functions
On Mon, 2018-09-10 at 07:42 -0600, Jan Beulich wrote:
> > > > On 10.09.18 at 15:33, wrote:
> >
> > On Mon, 2018-09-10 at 15:36 +0300, Alexandru Isaila wrote:
> > > This patch removes the redundant save functions and renames the
> > > save_one* to save. It then changes the domain param to vcpu in
> > > the
> > > save funcs and adapts print messages in order to match the format
> > > of
> > > the
> > > other save related messages.
> > >
> > > Signed-off-by: Alexandru Isaila
> > >
> > > ---
> > > Changes since V18:
> > > - Add const struct domain to rtc_save and hpet_save
> > > - Latched the vCPU into a local variable in hvm_save_one()
> > > - Add HVMSR_PER_VCPU kind check to the bounds if.
> > > ---
> > > xen/arch/x86/cpu/mcheck/vmce.c | 18 +---
> > > xen/arch/x86/emul-i8254.c | 5 ++-
> > > xen/arch/x86/hvm/hpet.c| 7 ++--
> > > xen/arch/x86/hvm/hvm.c | 75 +++-
> > >
> > > --
> > > xen/arch/x86/hvm/irq.c | 15 ---
> > > xen/arch/x86/hvm/mtrr.c| 22 ++
> > > xen/arch/x86/hvm/pmtimer.c | 5 ++-
> > > xen/arch/x86/hvm/rtc.c | 5 ++-
> > > xen/arch/x86/hvm/save.c| 28 +++--
> > > xen/arch/x86/hvm/vioapic.c | 5 ++-
> > > xen/arch/x86/hvm/viridian.c| 23 ++-
> > > xen/arch/x86/hvm/vlapic.c | 38 ++---
> > > xen/arch/x86/hvm/vpic.c| 5 ++-
> > > xen/include/asm-x86/hvm/save.h | 8 +---
> > > 14 files changed, 63 insertions(+), 196 deletions(-)
> > >
> > > @@ -141,6 +138,8 @@ int hvm_save_one(struct domain *d, unsigned
> > > int
> > > typecode, unsigned int instance,
> > > int rv;
> > > hvm_domain_context_t ctxt = { };
> > > const struct hvm_save_descriptor *desc;
> > > +struct vcpu *v = (hvm_sr_handlers[typecode].kind ==
> > > HVMSR_PER_VCPU) ?
> > > + d->vcpu[instance] : d->vcpu[0];
> > >
> >
> > Sorry for the inconvenience but I've just realized that this has to
> > be
> > initialize after the bounds check. I will have this in mine
>
> Also to eliminate redundancy I'd prefer if you moved the conditional
> expression inside the square brackets.
>
Are these changes worth waiting 24h?
Alex
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v19 12/13] x86/hvm: Remove redundant save functions
>>> On 10.09.18 at 15:33, wrote:
> On Mon, 2018-09-10 at 15:36 +0300, Alexandru Isaila wrote:
>> This patch removes the redundant save functions and renames the
>> save_one* to save. It then changes the domain param to vcpu in the
>> save funcs and adapts print messages in order to match the format of
>> the
>> other save related messages.
>>
>> Signed-off-by: Alexandru Isaila
>>
>> ---
>> Changes since V18:
>> - Add const struct domain to rtc_save and hpet_save
>> - Latched the vCPU into a local variable in hvm_save_one()
>> - Add HVMSR_PER_VCPU kind check to the bounds if.
>> ---
>> xen/arch/x86/cpu/mcheck/vmce.c | 18 +---
>> xen/arch/x86/emul-i8254.c | 5 ++-
>> xen/arch/x86/hvm/hpet.c| 7 ++--
>> xen/arch/x86/hvm/hvm.c | 75 +++-
>> --
>> xen/arch/x86/hvm/irq.c | 15 ---
>> xen/arch/x86/hvm/mtrr.c| 22 ++
>> xen/arch/x86/hvm/pmtimer.c | 5 ++-
>> xen/arch/x86/hvm/rtc.c | 5 ++-
>> xen/arch/x86/hvm/save.c| 28 +++--
>> xen/arch/x86/hvm/vioapic.c | 5 ++-
>> xen/arch/x86/hvm/viridian.c| 23 ++-
>> xen/arch/x86/hvm/vlapic.c | 38 ++---
>> xen/arch/x86/hvm/vpic.c| 5 ++-
>> xen/include/asm-x86/hvm/save.h | 8 +---
>> 14 files changed, 63 insertions(+), 196 deletions(-)
>>
>> @@ -141,6 +138,8 @@ int hvm_save_one(struct domain *d, unsigned int
>> typecode, unsigned int instance,
>> int rv;
>> hvm_domain_context_t ctxt = { };
>> const struct hvm_save_descriptor *desc;
>> +struct vcpu *v = (hvm_sr_handlers[typecode].kind ==
>> HVMSR_PER_VCPU) ?
>> + d->vcpu[instance] : d->vcpu[0];
>>
> Sorry for the inconvenience but I've just realized that this has to be
> initialize after the bounds check. I will have this in mine
Also to eliminate redundancy I'd prefer if you moved the conditional
expression inside the square brackets.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v19 12/13] x86/hvm: Remove redundant save functions
On Mon, 2018-09-10 at 15:36 +0300, Alexandru Isaila wrote:
> This patch removes the redundant save functions and renames the
> save_one* to save. It then changes the domain param to vcpu in the
> save funcs and adapts print messages in order to match the format of
> the
> other save related messages.
>
> Signed-off-by: Alexandru Isaila
>
> ---
> Changes since V18:
> - Add const struct domain to rtc_save and hpet_save
> - Latched the vCPU into a local variable in hvm_save_one()
> - Add HVMSR_PER_VCPU kind check to the bounds if.
> ---
> xen/arch/x86/cpu/mcheck/vmce.c | 18 +---
> xen/arch/x86/emul-i8254.c | 5 ++-
> xen/arch/x86/hvm/hpet.c| 7 ++--
> xen/arch/x86/hvm/hvm.c | 75 +++-
> --
> xen/arch/x86/hvm/irq.c | 15 ---
> xen/arch/x86/hvm/mtrr.c| 22 ++
> xen/arch/x86/hvm/pmtimer.c | 5 ++-
> xen/arch/x86/hvm/rtc.c | 5 ++-
> xen/arch/x86/hvm/save.c| 28 +++--
> xen/arch/x86/hvm/vioapic.c | 5 ++-
> xen/arch/x86/hvm/viridian.c| 23 ++-
> xen/arch/x86/hvm/vlapic.c | 38 ++---
> xen/arch/x86/hvm/vpic.c| 5 ++-
> xen/include/asm-x86/hvm/save.h | 8 +---
> 14 files changed, 63 insertions(+), 196 deletions(-)
>
> @@ -141,6 +138,8 @@ int hvm_save_one(struct domain *d, unsigned int
> typecode, unsigned int instance,
> int rv;
> hvm_domain_context_t ctxt = { };
> const struct hvm_save_descriptor *desc;
> +struct vcpu *v = (hvm_sr_handlers[typecode].kind ==
> HVMSR_PER_VCPU) ?
> + d->vcpu[instance] : d->vcpu[0];
>
Sorry for the inconvenience but I've just realized that this has to be
initialize after the bounds check. I will have this in mine
Thanks,
Alex
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v19 12/13] x86/hvm: Remove redundant save functions
This patch removes the redundant save functions and renames the
save_one* to save. It then changes the domain param to vcpu in the
save funcs and adapts print messages in order to match the format of the
other save related messages.
Signed-off-by: Alexandru Isaila
---
Changes since V18:
- Add const struct domain to rtc_save and hpet_save
- Latched the vCPU into a local variable in hvm_save_one()
- Add HVMSR_PER_VCPU kind check to the bounds if.
---
xen/arch/x86/cpu/mcheck/vmce.c | 18 +---
xen/arch/x86/emul-i8254.c | 5 ++-
xen/arch/x86/hvm/hpet.c| 7 ++--
xen/arch/x86/hvm/hvm.c | 75 +++---
xen/arch/x86/hvm/irq.c | 15 ---
xen/arch/x86/hvm/mtrr.c| 22 ++
xen/arch/x86/hvm/pmtimer.c | 5 ++-
xen/arch/x86/hvm/rtc.c | 5 ++-
xen/arch/x86/hvm/save.c| 28 +++--
xen/arch/x86/hvm/vioapic.c | 5 ++-
xen/arch/x86/hvm/viridian.c| 23 ++-
xen/arch/x86/hvm/vlapic.c | 38 ++---
xen/arch/x86/hvm/vpic.c| 5 ++-
xen/include/asm-x86/hvm/save.h | 8 +---
14 files changed, 63 insertions(+), 196 deletions(-)
diff --git a/xen/arch/x86/cpu/mcheck/vmce.c b/xen/arch/x86/cpu/mcheck/vmce.c
index 71afc06f9a..f15835e9f6 100644
--- a/xen/arch/x86/cpu/mcheck/vmce.c
+++ b/xen/arch/x86/cpu/mcheck/vmce.c
@@ -350,7 +350,7 @@ int vmce_wrmsr(uint32_t msr, uint64_t val)
}
#if CONFIG_HVM
-static int vmce_save_vcpu_ctxt_one(struct vcpu *v, hvm_domain_context_t *h)
+static int vmce_save_vcpu_ctxt(struct vcpu *v, hvm_domain_context_t *h)
{
struct hvm_vmce_vcpu ctxt = {
.caps = v->arch.vmce.mcg_cap,
@@ -362,21 +362,6 @@ static int vmce_save_vcpu_ctxt_one(struct vcpu *v,
hvm_domain_context_t *h)
return hvm_save_entry(VMCE_VCPU, v->vcpu_id, h, &ctxt);
}
-static int vmce_save_vcpu_ctxt(struct domain *d, hvm_domain_context_t *h)
-{
-struct vcpu *v;
-int err = 0;
-
-for_each_vcpu ( d, v )
-{
-err = vmce_save_vcpu_ctxt_one(v, h);
-if ( err )
-break;
-}
-
-return err;
-}
-
static int vmce_load_vcpu_ctxt(struct domain *d, hvm_domain_context_t *h)
{
unsigned int vcpuid = hvm_load_instance(h);
@@ -397,7 +382,6 @@ static int vmce_load_vcpu_ctxt(struct domain *d,
hvm_domain_context_t *h)
}
HVM_REGISTER_SAVE_RESTORE(VMCE_VCPU, vmce_save_vcpu_ctxt,
- vmce_save_vcpu_ctxt_one,
vmce_load_vcpu_ctxt, 1, HVMSR_PER_VCPU);
#endif
diff --git a/xen/arch/x86/emul-i8254.c b/xen/arch/x86/emul-i8254.c
index a85dfcccbc..73be4188ad 100644
--- a/xen/arch/x86/emul-i8254.c
+++ b/xen/arch/x86/emul-i8254.c
@@ -391,8 +391,9 @@ void pit_stop_channel0_irq(PITState *pit)
spin_unlock(&pit->lock);
}
-static int pit_save(struct domain *d, hvm_domain_context_t *h)
+static int pit_save(struct vcpu *v, hvm_domain_context_t *h)
{
+struct domain *d = v->domain;
PITState *pit = domain_vpit(d);
int rc;
@@ -438,7 +439,7 @@ static int pit_load(struct domain *d, hvm_domain_context_t
*h)
return 0;
}
-HVM_REGISTER_SAVE_RESTORE(PIT, pit_save, NULL, pit_load, 1, HVMSR_PER_DOM);
+HVM_REGISTER_SAVE_RESTORE(PIT, pit_save, pit_load, 1, HVMSR_PER_DOM);
#endif
void pit_reset(struct domain *d)
diff --git a/xen/arch/x86/hvm/hpet.c b/xen/arch/x86/hvm/hpet.c
index 4d8f6da2d9..be371ecc0b 100644
--- a/xen/arch/x86/hvm/hpet.c
+++ b/xen/arch/x86/hvm/hpet.c
@@ -570,16 +570,17 @@ static const struct hvm_mmio_ops hpet_mmio_ops = {
};
-static int hpet_save(struct domain *d, hvm_domain_context_t *h)
+static int hpet_save(struct vcpu *v, hvm_domain_context_t *h)
{
+const struct domain *d = v->domain;
HPETState *hp = domain_vhpet(d);
-struct vcpu *v = pt_global_vcpu_target(d);
int rc;
uint64_t guest_time;
if ( !has_vhpet(d) )
return 0;
+v = pt_global_vcpu_target(d);
write_lock(&hp->lock);
guest_time = (v->arch.hvm.guest_time ?: hvm_get_guest_time(v)) /
STIME_PER_HPET_TICK;
@@ -695,7 +696,7 @@ static int hpet_load(struct domain *d, hvm_domain_context_t
*h)
return 0;
}
-HVM_REGISTER_SAVE_RESTORE(HPET, hpet_save, NULL, hpet_load, 1, HVMSR_PER_DOM);
+HVM_REGISTER_SAVE_RESTORE(HPET, hpet_save, hpet_load, 1, HVMSR_PER_DOM);
static void hpet_set(HPETState *h)
{
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 58c03bed15..43145586c5 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -731,7 +731,7 @@ void hvm_domain_destroy(struct domain *d)
destroy_vpci_mmcfg(d);
}
-static int hvm_save_tsc_adjust_one(struct vcpu *v, hvm_domain_context_t *h)
+static int hvm_save_tsc_adjust(struct vcpu *v, hvm_domain_context_t *h)
{
struct hvm_tsc_adjust ctxt = {
.tsc_adjust = v->arch.hvm.msr_tsc_adjust,
@@ -740,21 +740,6 @@ static int hvm_save_tsc_adjust_one(struct vcpu *v,
hvm_domain_context_t *h)
return hvm_save_entry(TSC_ADJUST,
