| Message ID | 1466085998-7473-1-git-send-email-czuzu@bitdefender.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 16.06.16 at 16:06, <czuzu@bitdefender.com> wrote: > --- a/xen/arch/x86/hvm/event.c > +++ b/xen/arch/x86/hvm/event.c > @@ -23,6 +23,7 @@ > > #include <xen/vm_event.h> > #include <asm/hvm/event.h> > +#include <asm/paging.h> > #include <asm/monitor.h> > #include <asm/vm_event.h> > #include <public/vm_event.h> > diff --git a/xen/common/monitor.c b/xen/common/monitor.c > index d950a7c..b30857a 100644 > --- a/xen/common/monitor.c > +++ b/xen/common/monitor.c > @@ -22,7 +22,6 @@ > #include <xen/monitor.h> > #include <xen/sched.h> > #include <xsm/xsm.h> > -#include <public/domctl.h> > #include <asm/monitor.h> > #include <asm/vm_event.h> These two adjustments clearly don't fit title / description. I certainly don't mind unnecessary inclusions to be dropped, but the addition of one clearly needs explanation (after all thing build fine without it). > --- a/xen/include/asm-x86/vm_event.h > +++ b/xen/include/asm-x86/vm_event.h > @@ -44,6 +44,9 @@ void vm_event_set_registers(struct vcpu *v, vm_event_response_t *rsp); > > void vm_event_fill_regs(vm_event_request_t *req); > > +/* > + * Monitor vm-events. > + */ This is a single line comment (also elsewhere). > --- a/xen/include/xen/vm_event.h > +++ b/xen/include/xen/vm_event.h > @@ -85,7 +85,6 @@ void vm_event_monitor_guest_request(void); > > #endif /* __VM_EVENT_H__ */ > > - > /* > * Local variables: > * mode: C Why don't you remove the other stray blank line at once? Jan
> diff --git a/xen/include/asm-arm/vm_event.h b/xen/include/asm-arm/vm_event.h > index 014d9ba..05c3027 100644 > --- a/xen/include/asm-arm/vm_event.h > +++ b/xen/include/asm-arm/vm_event.h > @@ -23,21 +23,18 @@ > #include <xen/vm_event.h> > #include <public/domctl.h> > > -static inline > -int vm_event_init_domain(struct domain *d) > +static inline int vm_event_init_domain(struct domain *d) > { > /* Nothing to do. */ > return 0; > } > > -static inline > -void vm_event_cleanup_domain(struct domain *d) > +static inline void vm_event_cleanup_domain(struct domain *d) > { > memset(&d->monitor, 0, sizeof(d->monitor)); > } > > -static inline > -void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v) > +static inline void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v) > { > /* Not supported on ARM. */ > } > @@ -59,6 +56,9 @@ static inline void vm_event_fill_regs(vm_event_request_t *req) > /* Not supported on ARM. */ > } > > +/* > + * Monitor vm-events. > + */ I already have an acked patch that relocates monitor-related functions from here and the x86 header to the monitor subsystem (https://patchwork.kernel.org/patch/9139999/). Generally, I'm trying to keep monitor-related stuff in the appropriately named files, so if you encounter things like this in the future the best course of action is to relocate them. vm_event should be use-case neutral by not having specific things for the monitor subsystem and just be the i/o system used for passing messages. > static inline uint32_t vm_event_monitor_get_capabilities(struct domain *d) > { > uint32_t capabilities = 0;
On 6/16/2016 5:24 PM, Jan Beulich wrote: >>>> On 16.06.16 at 16:06, <czuzu@bitdefender.com> wrote: >> --- a/xen/arch/x86/hvm/event.c >> +++ b/xen/arch/x86/hvm/event.c >> @@ -23,6 +23,7 @@ >> >> #include <xen/vm_event.h> >> #include <asm/hvm/event.h> >> +#include <asm/paging.h> >> #include <asm/monitor.h> >> #include <asm/vm_event.h> >> #include <public/vm_event.h> >> diff --git a/xen/common/monitor.c b/xen/common/monitor.c >> index d950a7c..b30857a 100644 >> --- a/xen/common/monitor.c >> +++ b/xen/common/monitor.c >> @@ -22,7 +22,6 @@ >> #include <xen/monitor.h> >> #include <xen/sched.h> >> #include <xsm/xsm.h> >> -#include <public/domctl.h> >> #include <asm/monitor.h> >> #include <asm/vm_event.h> > These two adjustments clearly don't fit title / description. I certainly > don't mind unnecessary inclusions to be dropped, but the addition of > one clearly needs explanation (after all thing build fine without it). Sorry, that was done out of reflex, should have stated the reasoning. Generally, if: - event.c includes event.h - event.c needs paging.h - event.h -doesn't need- paging.h then I prefer to include paging.h in event.c, not in event.h (include strictly -where- needed). Also since xen/paging.h included asm/paging.h and event.c only needs the asm/paging.h part, I also changed xen/paging.h inclusion -> asm/paging.h inclusion (include strictly -what's- needed). But I can revert that if you want me to (not that important and these little things are better done with automatic tools anyway), or should I leave the change and update the commit message? >> --- a/xen/include/asm-x86/vm_event.h >> +++ b/xen/include/asm-x86/vm_event.h >> @@ -44,6 +44,9 @@ void vm_event_set_registers(struct vcpu *v, vm_event_response_t *rsp); >> >> void vm_event_fill_regs(vm_event_request_t *req); >> >> +/* >> + * Monitor vm-events. >> + */ > This is a single line comment (also elsewhere). Ack. > >> --- a/xen/include/xen/vm_event.h >> +++ b/xen/include/xen/vm_event.h >> @@ -85,7 +85,6 @@ void vm_event_monitor_guest_request(void); >> >> #endif /* __VM_EVENT_H__ */ >> >> - >> /* >> * Local variables: >> * mode: C > Why don't you remove the other stray blank line at once? > > Jan What stray line? Shouldn't there be -one- blank line between the #endif and the local vars block? Looking @ other files that rule seems to hold... Corneliu.
>>> On 16.06.16 at 21:19, <czuzu@bitdefender.com> wrote: > On 6/16/2016 5:24 PM, Jan Beulich wrote: >>>>> On 16.06.16 at 16:06, <czuzu@bitdefender.com> wrote: >>> --- a/xen/arch/x86/hvm/event.c >>> +++ b/xen/arch/x86/hvm/event.c >>> @@ -23,6 +23,7 @@ >>> >>> #include <xen/vm_event.h> >>> #include <asm/hvm/event.h> >>> +#include <asm/paging.h> >>> #include <asm/monitor.h> >>> #include <asm/vm_event.h> >>> #include <public/vm_event.h> >>> diff --git a/xen/common/monitor.c b/xen/common/monitor.c >>> index d950a7c..b30857a 100644 >>> --- a/xen/common/monitor.c >>> +++ b/xen/common/monitor.c >>> @@ -22,7 +22,6 @@ >>> #include <xen/monitor.h> >>> #include <xen/sched.h> >>> #include <xsm/xsm.h> >>> -#include <public/domctl.h> >>> #include <asm/monitor.h> >>> #include <asm/vm_event.h> >> These two adjustments clearly don't fit title / description. I certainly >> don't mind unnecessary inclusions to be dropped, but the addition of >> one clearly needs explanation (after all thing build fine without it). > > Sorry, that was done out of reflex, should have stated the reasoning. > Generally, if: > - event.c includes event.h > - event.c needs paging.h > - event.h -doesn't need- paging.h > then I prefer to include paging.h in event.c, not in event.h (include > strictly -where- needed). > Also since xen/paging.h included asm/paging.h and event.c only needs the > asm/paging.h part, I also changed xen/paging.h inclusion -> asm/paging.h > inclusion (include strictly -what's- needed). > But I can revert that if you want me to (not that important and these > little things are better done with automatic tools anyway), or should I > leave the change and update the commit message? Well, I'd leave the ultimate decision to the maintainers, but I'd say this doesn't belong in a patch dealing with formatting issues. I.e. I'm fine with the cleanup, but either under a different title (and with at least an abbreviated version of what you said above), or in a separate patch. Considering that you appear to do the same in later patches, perhaps consolidating all the #include adjustments into one patch would a good idea? >>> --- a/xen/include/xen/vm_event.h >>> +++ b/xen/include/xen/vm_event.h >>> @@ -85,7 +85,6 @@ void vm_event_monitor_guest_request(void); >>> >>> #endif /* __VM_EVENT_H__ */ >>> >>> - >>> /* >>> * Local variables: >>> * mode: C >> Why don't you remove the other stray blank line at once? > > What stray line? Shouldn't there be -one- blank line between the #endif > and the local vars block? > Looking @ other files that rule seems to hold... Oh, you're right. I thought I had frequently seen it the other way around (and it would seem more logical to me), but I must have been misremembering. Jan
On 6/16/2016 7:02 PM, Tamas K Lengyel wrote: >> diff --git a/xen/include/asm-arm/vm_event.h b/xen/include/asm-arm/vm_event.h >> index 014d9ba..05c3027 100644 >> --- a/xen/include/asm-arm/vm_event.h >> +++ b/xen/include/asm-arm/vm_event.h >> @@ -23,21 +23,18 @@ >> #include <xen/vm_event.h> >> #include <public/domctl.h> >> >> -static inline >> -int vm_event_init_domain(struct domain *d) >> +static inline int vm_event_init_domain(struct domain *d) >> { >> /* Nothing to do. */ >> return 0; >> } >> >> -static inline >> -void vm_event_cleanup_domain(struct domain *d) >> +static inline void vm_event_cleanup_domain(struct domain *d) >> { >> memset(&d->monitor, 0, sizeof(d->monitor)); >> } >> >> -static inline >> -void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v) >> +static inline void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v) >> { >> /* Not supported on ARM. */ >> } >> @@ -59,6 +56,9 @@ static inline void vm_event_fill_regs(vm_event_request_t *req) >> /* Not supported on ARM. */ >> } >> >> +/* >> + * Monitor vm-events. >> + */ > I already have an acked patch that relocates monitor-related functions > from here and the x86 header to the monitor subsystem > (https://patchwork.kernel.org/patch/9139999/). Generally, I'm trying > to keep monitor-related stuff in the appropriately named files, so if > you encounter things like this in the future the best course of action > is to relocate them. vm_event should be use-case neutral by not having > specific things for the monitor subsystem and just be the i/o system > used for passing messages. > >> static inline uint32_t vm_event_monitor_get_capabilities(struct domain *d) >> { >> uint32_t capabilities = 0; Ah, ok. Didn't that patch make it to staging yet? I pulled the latest. Since you already have a patch for that I guess it's ok to remove those comments and leave the rest as it is and merge later when one of these patches makes it to staging? Corneliu.
On 06/17/2016 11:33 AM, Corneliu ZUZU wrote: > On 6/16/2016 7:02 PM, Tamas K Lengyel wrote: >>> diff --git a/xen/include/asm-arm/vm_event.h >>> b/xen/include/asm-arm/vm_event.h >>> index 014d9ba..05c3027 100644 >>> --- a/xen/include/asm-arm/vm_event.h >>> +++ b/xen/include/asm-arm/vm_event.h >>> @@ -23,21 +23,18 @@ >>> #include <xen/vm_event.h> >>> #include <public/domctl.h> >>> >>> -static inline >>> -int vm_event_init_domain(struct domain *d) >>> +static inline int vm_event_init_domain(struct domain *d) >>> { >>> /* Nothing to do. */ >>> return 0; >>> } >>> >>> -static inline >>> -void vm_event_cleanup_domain(struct domain *d) >>> +static inline void vm_event_cleanup_domain(struct domain *d) >>> { >>> memset(&d->monitor, 0, sizeof(d->monitor)); >>> } >>> >>> -static inline >>> -void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v) >>> +static inline void vm_event_toggle_singlestep(struct domain *d, >>> struct vcpu *v) >>> { >>> /* Not supported on ARM. */ >>> } >>> @@ -59,6 +56,9 @@ static inline void >>> vm_event_fill_regs(vm_event_request_t *req) >>> /* Not supported on ARM. */ >>> } >>> >>> +/* >>> + * Monitor vm-events. >>> + */ >> I already have an acked patch that relocates monitor-related functions >> from here and the x86 header to the monitor subsystem >> (https://patchwork.kernel.org/patch/9139999/). Generally, I'm trying >> to keep monitor-related stuff in the appropriately named files, so if >> you encounter things like this in the future the best course of action >> is to relocate them. vm_event should be use-case neutral by not having >> specific things for the monitor subsystem and just be the i/o system >> used for passing messages. >> >>> static inline uint32_t vm_event_monitor_get_capabilities(struct >>> domain *d) >>> { >>> uint32_t capabilities = 0; > > Ah, ok. Didn't that patch make it to staging yet? I pulled the latest. > Since you already have a patch for that I guess it's ok to remove those > comments and leave the rest as it is and merge later when one of these > patches makes it to staging? I think there's a delay caused by the 4.7 release. I also have an acked patch waiting to go in that might cause you to have to rebase (part of) the series: https://patchwork.kernel.org/patch/9033771/ Thanks, Razvan
On 17/06/16 09:36, Razvan Cojocaru wrote: > On 06/17/2016 11:33 AM, Corneliu ZUZU wrote: >> On 6/16/2016 7:02 PM, Tamas K Lengyel wrote: >>>> diff --git a/xen/include/asm-arm/vm_event.h >>>> b/xen/include/asm-arm/vm_event.h >>>> index 014d9ba..05c3027 100644 >>>> --- a/xen/include/asm-arm/vm_event.h >>>> +++ b/xen/include/asm-arm/vm_event.h >>>> @@ -23,21 +23,18 @@ >>>> #include <xen/vm_event.h> >>>> #include <public/domctl.h> >>>> >>>> -static inline >>>> -int vm_event_init_domain(struct domain *d) >>>> +static inline int vm_event_init_domain(struct domain *d) >>>> { >>>> /* Nothing to do. */ >>>> return 0; >>>> } >>>> >>>> -static inline >>>> -void vm_event_cleanup_domain(struct domain *d) >>>> +static inline void vm_event_cleanup_domain(struct domain *d) >>>> { >>>> memset(&d->monitor, 0, sizeof(d->monitor)); >>>> } >>>> >>>> -static inline >>>> -void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v) >>>> +static inline void vm_event_toggle_singlestep(struct domain *d, >>>> struct vcpu *v) >>>> { >>>> /* Not supported on ARM. */ >>>> } >>>> @@ -59,6 +56,9 @@ static inline void >>>> vm_event_fill_regs(vm_event_request_t *req) >>>> /* Not supported on ARM. */ >>>> } >>>> >>>> +/* >>>> + * Monitor vm-events. >>>> + */ >>> I already have an acked patch that relocates monitor-related functions >>> from here and the x86 header to the monitor subsystem >>> (https://patchwork.kernel.org/patch/9139999/). Generally, I'm trying >>> to keep monitor-related stuff in the appropriately named files, so if >>> you encounter things like this in the future the best course of action >>> is to relocate them. vm_event should be use-case neutral by not having >>> specific things for the monitor subsystem and just be the i/o system >>> used for passing messages. >>> >>>> static inline uint32_t vm_event_monitor_get_capabilities(struct >>>> domain *d) >>>> { >>>> uint32_t capabilities = 0; >> Ah, ok. Didn't that patch make it to staging yet? I pulled the latest. >> Since you already have a patch for that I guess it's ok to remove those >> comments and leave the rest as it is and merge later when one of these >> patches makes it to staging? > I think there's a delay caused by the 4.7 release. I also have an acked > patch waiting to go in that might cause you to have to rebase (part of) > the series: https://patchwork.kernel.org/patch/9033771/ Staging is now open for Xen 4.8 submissions. Would it be possible for previous submissions to be resent, or a summary email pointing at the intended versions of the patches? I will see about getting stuff in. ~Andrew
>>> On 17.06.16 at 10:36, <rcojocaru@bitdefender.com> wrote: > On 06/17/2016 11:33 AM, Corneliu ZUZU wrote: >> Ah, ok. Didn't that patch make it to staging yet? I pulled the latest. >> Since you already have a patch for that I guess it's ok to remove those >> comments and leave the rest as it is and merge later when one of these >> patches makes it to staging? > > I think there's a delay caused by the 4.7 release. I also have an acked > patch waiting to go in that might cause you to have to rebase (part of) > the series: https://patchwork.kernel.org/patch/9033771/ Well, there's a slight difference here: Yours is indeed delayed until 4.7 is set in stone, for being more intrusive. Tamas'es, otoh, I would probably have put in already if it had all necessary acks (or they had at least been pinged for, at which point we could waive the need for some trivial adjustments). Jan
>>> On 17.06.16 at 11:29, <andrew.cooper3@citrix.com> wrote: > Staging is now open for Xen 4.8 submissions. Just to clarify - not fully. Intrusive changes (with a higher risk of causing backporting conflicts for eventual changes still wanting to go into 4.7) were agreed to not get put in yet. Jan
On 06/17/2016 12:33 PM, Jan Beulich wrote: >>>> On 17.06.16 at 10:36, <rcojocaru@bitdefender.com> wrote: >> On 06/17/2016 11:33 AM, Corneliu ZUZU wrote: >>> Ah, ok. Didn't that patch make it to staging yet? I pulled the latest. >>> Since you already have a patch for that I guess it's ok to remove those >>> comments and leave the rest as it is and merge later when one of these >>> patches makes it to staging? >> >> I think there's a delay caused by the 4.7 release. I also have an acked >> patch waiting to go in that might cause you to have to rebase (part of) >> the series: https://patchwork.kernel.org/patch/9033771/ > > Well, there's a slight difference here: Yours is indeed delayed until > 4.7 is set in stone, for being more intrusive. Tamas'es, otoh, I would > probably have put in already if it had all necessary acks (or they > had at least been pinged for, at which point we could waive the need > for some trivial adjustments). Right, so should I resend as Andrew has requested, or just wait? Thanks, Razvan
>>> On 17.06.16 at 11:36, <rcojocaru@bitdefender.com> wrote: > On 06/17/2016 12:33 PM, Jan Beulich wrote: >>>>> On 17.06.16 at 10:36, <rcojocaru@bitdefender.com> wrote: >>> On 06/17/2016 11:33 AM, Corneliu ZUZU wrote: >>>> Ah, ok. Didn't that patch make it to staging yet? I pulled the latest. >>>> Since you already have a patch for that I guess it's ok to remove those >>>> comments and leave the rest as it is and merge later when one of these >>>> patches makes it to staging? >>> >>> I think there's a delay caused by the 4.7 release. I also have an acked >>> patch waiting to go in that might cause you to have to rebase (part of) >>> the series: https://patchwork.kernel.org/patch/9033771/ >> >> Well, there's a slight difference here: Yours is indeed delayed until >> 4.7 is set in stone, for being more intrusive. Tamas'es, otoh, I would >> probably have put in already if it had all necessary acks (or they >> had at least been pinged for, at which point we could waive the need >> for some trivial adjustments). > > Right, so should I resend as Andrew has requested, or just wait? I have the patch queued, so unless it needs rebasing I don't see a strict need for resending. Jan
On 06/17/2016 12:40 PM, Jan Beulich wrote: >>>> On 17.06.16 at 11:36, <rcojocaru@bitdefender.com> wrote: >> On 06/17/2016 12:33 PM, Jan Beulich wrote: >>>>>> On 17.06.16 at 10:36, <rcojocaru@bitdefender.com> wrote: >>>> On 06/17/2016 11:33 AM, Corneliu ZUZU wrote: >>>>> Ah, ok. Didn't that patch make it to staging yet? I pulled the latest. >>>>> Since you already have a patch for that I guess it's ok to remove those >>>>> comments and leave the rest as it is and merge later when one of these >>>>> patches makes it to staging? >>>> >>>> I think there's a delay caused by the 4.7 release. I also have an acked >>>> patch waiting to go in that might cause you to have to rebase (part of) >>>> the series: https://patchwork.kernel.org/patch/9033771/ >>> >>> Well, there's a slight difference here: Yours is indeed delayed until >>> 4.7 is set in stone, for being more intrusive. Tamas'es, otoh, I would >>> probably have put in already if it had all necessary acks (or they >>> had at least been pinged for, at which point we could waive the need >>> for some trivial adjustments). >> >> Right, so should I resend as Andrew has requested, or just wait? > > I have the patch queued, so unless it needs rebasing I don't see a > strict need for resending. Checking just now, it doesn't seem to need rebasing. Thanks, Razvan
On 6/17/2016 10:06 AM, Jan Beulich wrote: >>>> On 16.06.16 at 21:19, <czuzu@bitdefender.com> wrote: >> On 6/16/2016 5:24 PM, Jan Beulich wrote: >>>>>> On 16.06.16 at 16:06, <czuzu@bitdefender.com> wrote: >>>> --- a/xen/arch/x86/hvm/event.c >>>> +++ b/xen/arch/x86/hvm/event.c >>>> @@ -23,6 +23,7 @@ >>>> >>>> #include <xen/vm_event.h> >>>> #include <asm/hvm/event.h> >>>> +#include <asm/paging.h> >>>> #include <asm/monitor.h> >>>> #include <asm/vm_event.h> >>>> #include <public/vm_event.h> >>>> diff --git a/xen/common/monitor.c b/xen/common/monitor.c >>>> index d950a7c..b30857a 100644 >>>> --- a/xen/common/monitor.c >>>> +++ b/xen/common/monitor.c >>>> @@ -22,7 +22,6 @@ >>>> #include <xen/monitor.h> >>>> #include <xen/sched.h> >>>> #include <xsm/xsm.h> >>>> -#include <public/domctl.h> >>>> #include <asm/monitor.h> >>>> #include <asm/vm_event.h> >>> These two adjustments clearly don't fit title / description. I certainly >>> don't mind unnecessary inclusions to be dropped, but the addition of >>> one clearly needs explanation (after all thing build fine without it). >> Sorry, that was done out of reflex, should have stated the reasoning. >> Generally, if: >> - event.c includes event.h >> - event.c needs paging.h >> - event.h -doesn't need- paging.h >> then I prefer to include paging.h in event.c, not in event.h (include >> strictly -where- needed). >> Also since xen/paging.h included asm/paging.h and event.c only needs the >> asm/paging.h part, I also changed xen/paging.h inclusion -> asm/paging.h >> inclusion (include strictly -what's- needed). >> But I can revert that if you want me to (not that important and these >> little things are better done with automatic tools anyway), or should I >> leave the change and update the commit message? > Well, I'd leave the ultimate decision to the maintainers, but I'd say > this doesn't belong in a patch dealing with formatting issues. I.e. > I'm fine with the cleanup, but either under a different title (and > with at least an abbreviated version of what you said above), or > in a separate patch. Considering that you appear to do the same > in later patches, perhaps consolidating all the #include adjustments > into one patch would a good idea? I tend to prefer organizing what's easy to understand/minor first (to let reviewers get rid of those in a flash), so I'd prefer to leave it in this patch with the other minor fixes and update the commit message instead. But either way it's fine by me. >>>> --- a/xen/include/xen/vm_event.h >>>> +++ b/xen/include/xen/vm_event.h >>>> @@ -85,7 +85,6 @@ void vm_event_monitor_guest_request(void); >>>> >>>> #endif /* __VM_EVENT_H__ */ >>>> >>>> - >>>> /* >>>> * Local variables: >>>> * mode: C >>> Why don't you remove the other stray blank line at once? >> What stray line? Shouldn't there be -one- blank line between the #endif >> and the local vars block? >> Looking @ other files that rule seems to hold... > Oh, you're right. I thought I had frequently seen it the other way > around (and it would seem more logical to me), but I must have been > misremembering. > > Jan > > Thanks, Corneliu.
On Fri, Jun 17, 2016 at 3:33 AM, Jan Beulich <JBeulich@suse.com> wrote: >>>> On 17.06.16 at 10:36, <rcojocaru@bitdefender.com> wrote: >> On 06/17/2016 11:33 AM, Corneliu ZUZU wrote: >>> Ah, ok. Didn't that patch make it to staging yet? I pulled the latest. >>> Since you already have a patch for that I guess it's ok to remove those >>> comments and leave the rest as it is and merge later when one of these >>> patches makes it to staging? >> >> I think there's a delay caused by the 4.7 release. I also have an acked >> patch waiting to go in that might cause you to have to rebase (part of) >> the series: https://patchwork.kernel.org/patch/9033771/ > > Well, there's a slight difference here: Yours is indeed delayed until > 4.7 is set in stone, for being more intrusive. Tamas'es, otoh, I would > probably have put in already if it had all necessary acks (or they > had at least been pinged for, at which point we could waive the need > for some trivial adjustments). Patches 1 through 4 of my v5 series already has acks, so I guess I'll issue a ping for the other maintainers to check. Tamas
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c index 1365b4a..d31f821 100644 --- a/xen/arch/arm/domain.c +++ b/xen/arch/arm/domain.c @@ -274,7 +274,6 @@ static void continue_new_vcpu(struct vcpu *prev) else /* check_wakeup_from_wait(); */ reset_stack_and_jump(return_to_new_vcpu64); - } void context_switch(struct vcpu *prev, struct vcpu *next) diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c index aa3e3c2..7fa2ae5 100644 --- a/xen/arch/arm/traps.c +++ b/xen/arch/arm/traps.c @@ -439,7 +439,6 @@ static void inject_abt32_exception(struct cpu_user_regs *regs, far |= addr << 32; WRITE_SYSREG(far, FAR_EL1); WRITE_SYSREG(fsr, IFSR32_EL2); - #endif } else diff --git a/xen/arch/x86/hvm/event.c b/xen/arch/x86/hvm/event.c index 56c5514..9c51890 100644 --- a/xen/arch/x86/hvm/event.c +++ b/xen/arch/x86/hvm/event.c @@ -23,6 +23,7 @@ #include <xen/vm_event.h> #include <asm/hvm/event.h> +#include <asm/paging.h> #include <asm/monitor.h> #include <asm/vm_event.h> #include <public/vm_event.h> diff --git a/xen/common/monitor.c b/xen/common/monitor.c index d950a7c..b30857a 100644 --- a/xen/common/monitor.c +++ b/xen/common/monitor.c @@ -22,7 +22,6 @@ #include <xen/monitor.h> #include <xen/sched.h> #include <xsm/xsm.h> -#include <public/domctl.h> #include <asm/monitor.h> #include <asm/vm_event.h> diff --git a/xen/include/asm-arm/vm_event.h b/xen/include/asm-arm/vm_event.h index 014d9ba..05c3027 100644 --- a/xen/include/asm-arm/vm_event.h +++ b/xen/include/asm-arm/vm_event.h @@ -23,21 +23,18 @@ #include <xen/vm_event.h> #include <public/domctl.h> -static inline -int vm_event_init_domain(struct domain *d) +static inline int vm_event_init_domain(struct domain *d) { /* Nothing to do. */ return 0; } -static inline -void vm_event_cleanup_domain(struct domain *d) +static inline void vm_event_cleanup_domain(struct domain *d) { memset(&d->monitor, 0, sizeof(d->monitor)); } -static inline -void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v) +static inline void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v) { /* Not supported on ARM. */ } @@ -59,6 +56,9 @@ static inline void vm_event_fill_regs(vm_event_request_t *req) /* Not supported on ARM. */ } +/* + * Monitor vm-events. + */ static inline uint32_t vm_event_monitor_get_capabilities(struct domain *d) { uint32_t capabilities = 0; diff --git a/xen/include/asm-x86/hvm/event.h b/xen/include/asm-x86/hvm/event.h index 03f7fee..504bd66 100644 --- a/xen/include/asm-x86/hvm/event.h +++ b/xen/include/asm-x86/hvm/event.h @@ -19,8 +19,6 @@ #ifndef __ASM_X86_HVM_EVENT_H__ #define __ASM_X86_HVM_EVENT_H__ -#include <xen/sched.h> -#include <xen/paging.h> #include <public/vm_event.h> enum hvm_event_breakpoint_type diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h index d367099..7a662f9 100644 --- a/xen/include/asm-x86/monitor.h +++ b/xen/include/asm-x86/monitor.h @@ -23,9 +23,6 @@ #define __ASM_X86_MONITOR_H__ #include <xen/sched.h> -#include <public/domctl.h> -#include <asm/cpufeature.h> -#include <asm/hvm/hvm.h> #define monitor_ctrlreg_bitmask(ctrlreg_index) (1U << (ctrlreg_index)) diff --git a/xen/include/asm-x86/vm_event.h b/xen/include/asm-x86/vm_event.h index 0470240..df8e98d 100644 --- a/xen/include/asm-x86/vm_event.h +++ b/xen/include/asm-x86/vm_event.h @@ -44,6 +44,9 @@ void vm_event_set_registers(struct vcpu *v, vm_event_response_t *rsp); void vm_event_fill_regs(vm_event_request_t *req); +/* + * Monitor vm-events. + */ static inline uint32_t vm_event_monitor_get_capabilities(struct domain *d) { uint32_t capabilities = 0; diff --git a/xen/include/public/vm_event.h b/xen/include/public/vm_event.h index 9270d52..586f43b 100644 --- a/xen/include/public/vm_event.h +++ b/xen/include/public/vm_event.h @@ -74,19 +74,19 @@ * VM_EVENT_FLAG_SET_EMUL_READ_DATA are set, only the latter will be honored). */ #define VM_EVENT_FLAG_SET_EMUL_READ_DATA (1 << 5) - /* - * Deny completion of the operation that triggered the event. - * Currently only useful for MSR, CR0, CR3 and CR4 write events. - */ +/* + * Deny completion of the operation that triggered the event. + * Currently only useful for MSR and control-register write events. + */ #define VM_EVENT_FLAG_DENY (1 << 6) /* * This flag can be set in a request or a response * - * On a request, indicates that the event occurred in the alternate p2m specified by - * the altp2m_idx request field. + * On a request, indicates that the event occurred in the alternate p2m + * specified by the altp2m_idx request field. * - * On a response, indicates that the VCPU should resume in the alternate p2m specified - * by the altp2m_idx response field if possible. + * On a response, indicates that the VCPU should resume in the alternate p2m + * specified by the altp2m_idx response field if possible. */ #define VM_EVENT_FLAG_ALTERNATE_P2M (1 << 7) /* @@ -177,16 +177,16 @@ struct vm_event_regs_x86 { * FAULT_WITH_GLA: If the violation was triggered by accessing gla * FAULT_IN_GPT: If the violation was triggered during translating gla */ -#define MEM_ACCESS_R (1 << 0) -#define MEM_ACCESS_W (1 << 1) -#define MEM_ACCESS_X (1 << 2) -#define MEM_ACCESS_RWX (MEM_ACCESS_R | MEM_ACCESS_W | MEM_ACCESS_X) -#define MEM_ACCESS_RW (MEM_ACCESS_R | MEM_ACCESS_W) -#define MEM_ACCESS_RX (MEM_ACCESS_R | MEM_ACCESS_X) -#define MEM_ACCESS_WX (MEM_ACCESS_W | MEM_ACCESS_X) -#define MEM_ACCESS_GLA_VALID (1 << 3) -#define MEM_ACCESS_FAULT_WITH_GLA (1 << 4) -#define MEM_ACCESS_FAULT_IN_GPT (1 << 5) +#define MEM_ACCESS_R (1 << 0) +#define MEM_ACCESS_W (1 << 1) +#define MEM_ACCESS_X (1 << 2) +#define MEM_ACCESS_RWX (MEM_ACCESS_R | MEM_ACCESS_W | MEM_ACCESS_X) +#define MEM_ACCESS_RW (MEM_ACCESS_R | MEM_ACCESS_W) +#define MEM_ACCESS_RX (MEM_ACCESS_R | MEM_ACCESS_X) +#define MEM_ACCESS_WX (MEM_ACCESS_W | MEM_ACCESS_X) +#define MEM_ACCESS_GLA_VALID (1 << 3) +#define MEM_ACCESS_FAULT_WITH_GLA (1 << 4) +#define MEM_ACCESS_FAULT_IN_GPT (1 << 5) struct vm_event_mem_access { uint64_t gfn; diff --git a/xen/include/xen/vm_event.h b/xen/include/xen/vm_event.h index beda9fe..a10ee40 100644 --- a/xen/include/xen/vm_event.h +++ b/xen/include/xen/vm_event.h @@ -85,7 +85,6 @@ void vm_event_monitor_guest_request(void); #endif /* __VM_EVENT_H__ */ - /* * Local variables: * mode: C
Minor fixes: - fix 80-columns formatting in some places - remove some empty lines - remove some unused includes - add 2 comments Signed-off-by: Corneliu ZUZU <czuzu@bitdefender.com> --- xen/arch/arm/domain.c | 1 - xen/arch/arm/traps.c | 1 - xen/arch/x86/hvm/event.c | 1 + xen/common/monitor.c | 1 - xen/include/asm-arm/vm_event.h | 12 ++++++------ xen/include/asm-x86/hvm/event.h | 2 -- xen/include/asm-x86/monitor.h | 3 --- xen/include/asm-x86/vm_event.h | 3 +++ xen/include/public/vm_event.h | 36 ++++++++++++++++++------------------ xen/include/xen/vm_event.h | 1 - 10 files changed, 28 insertions(+), 33 deletions(-)