| Message ID | 20250414154838.556265-3-ggala@linux.ibm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | DIAG 308: extend subcode 10 to return UVC cmd id, RC and RRC values upon failure to enter secure mode | expand |
On Mon, Apr 14, 2025 at 05:48:37PM +0200, Gautam Gala wrote: > introduce a static function when exiting PV. The function replaces an > existing macro (s390_pv_cmd_exit). > Reviewed-by: Steffen Eiden <seiden@linux.ibm.com> > Signed-off-by: Gautam Gala <ggala@linux.ibm.com> Nit: start with capital letters at beginning of sentences and after a colon. ...
On 14/04/2025 17.48, Gautam Gala wrote: > introduce a static function when exiting PV. The function replaces an > existing macro (s390_pv_cmd_exit). You describe here what you're doing, but not why ... so may I ask: Why is this change necessary? Thomas > Signed-off-by: Gautam Gala <ggala@linux.ibm.com> > --- > target/s390x/kvm/pv.c | 14 ++++++-------- > 1 file changed, 6 insertions(+), 8 deletions(-) > > diff --git a/target/s390x/kvm/pv.c b/target/s390x/kvm/pv.c > index 3a0a971f0b..66194caaae 100644 > --- a/target/s390x/kvm/pv.c > +++ b/target/s390x/kvm/pv.c > @@ -59,14 +59,12 @@ static int __s390_pv_cmd(uint32_t cmd, const char *cmdname, void *data, > */ > #define s390_pv_cmd(cmd, data) __s390_pv_cmd(cmd, #cmd, data, NULL) > #define s390_pv_cmd_pvrc(cmd, data, pvrc) __s390_pv_cmd(cmd, #cmd, data, pvrc) > -#define s390_pv_cmd_exit(cmd, data) \ > -{ \ > - int rc; \ > - \ > - rc = __s390_pv_cmd(cmd, #cmd, data, NULL); \ > - if (rc) { \ > - exit(1); \ > - } \ > + > +static void s390_pv_cmd_exit(uint32_t cmd, void *data) > +{ > + if (s390_pv_cmd(cmd, data)) { > + exit(1); > + } > } > > int s390_pv_query_info(void)
Hi, The change is not really necessary. The existing macro looked almost like a function, and since I was making changes in the area, it felt like a good opportunity to change it to an actual function. Thanks, Gautam On Tue, Apr 15, 2025 at 11:44:35AM +0200, Thomas Huth wrote: > On 14/04/2025 17.48, Gautam Gala wrote: > > introduce a static function when exiting PV. The function replaces an > > existing macro (s390_pv_cmd_exit). > > You describe here what you're doing, but not why ... so may I ask: Why is > this change necessary? > > Thomas > > > > Signed-off-by: Gautam Gala <ggala@linux.ibm.com> > > --- > > target/s390x/kvm/pv.c | 14 ++++++-------- > > 1 file changed, 6 insertions(+), 8 deletions(-) > > > > diff --git a/target/s390x/kvm/pv.c b/target/s390x/kvm/pv.c > > index 3a0a971f0b..66194caaae 100644 > > --- a/target/s390x/kvm/pv.c > > +++ b/target/s390x/kvm/pv.c > > @@ -59,14 +59,12 @@ static int __s390_pv_cmd(uint32_t cmd, const char *cmdname, void *data, > > */ > > #define s390_pv_cmd(cmd, data) __s390_pv_cmd(cmd, #cmd, data, NULL) > > #define s390_pv_cmd_pvrc(cmd, data, pvrc) __s390_pv_cmd(cmd, #cmd, data, pvrc) > > -#define s390_pv_cmd_exit(cmd, data) \ > > -{ \ > > - int rc; \ > > - \ > > - rc = __s390_pv_cmd(cmd, #cmd, data, NULL); \ > > - if (rc) { \ > > - exit(1); \ > > - } \ > > + > > +static void s390_pv_cmd_exit(uint32_t cmd, void *data) > > +{ > > + if (s390_pv_cmd(cmd, data)) { > > + exit(1); > > + } > > } > > int s390_pv_query_info(void) > >
On 15/04/2025 14.04, Gautam Gala wrote: > Hi, > > The change is not really necessary. The existing macro looked almost like a > function, and since I was making changes in the area, it felt like a good > opportunity to change it to an actual function. Ok, makes sense, but please add that information to the patch description! Thanks, Thomas > > On Tue, Apr 15, 2025 at 11:44:35AM +0200, Thomas Huth wrote: >> On 14/04/2025 17.48, Gautam Gala wrote: >>> introduce a static function when exiting PV. The function replaces an >>> existing macro (s390_pv_cmd_exit). >> >> You describe here what you're doing, but not why ... so may I ask: Why is >> this change necessary? >> >> Thomas >> >> >>> Signed-off-by: Gautam Gala <ggala@linux.ibm.com> >>> --- >>> target/s390x/kvm/pv.c | 14 ++++++-------- >>> 1 file changed, 6 insertions(+), 8 deletions(-) >>> >>> diff --git a/target/s390x/kvm/pv.c b/target/s390x/kvm/pv.c >>> index 3a0a971f0b..66194caaae 100644 >>> --- a/target/s390x/kvm/pv.c >>> +++ b/target/s390x/kvm/pv.c >>> @@ -59,14 +59,12 @@ static int __s390_pv_cmd(uint32_t cmd, const char *cmdname, void *data, >>> */ >>> #define s390_pv_cmd(cmd, data) __s390_pv_cmd(cmd, #cmd, data, NULL) >>> #define s390_pv_cmd_pvrc(cmd, data, pvrc) __s390_pv_cmd(cmd, #cmd, data, pvrc) >>> -#define s390_pv_cmd_exit(cmd, data) \ >>> -{ \ >>> - int rc; \ >>> - \ >>> - rc = __s390_pv_cmd(cmd, #cmd, data, NULL); \ >>> - if (rc) { \ >>> - exit(1); \ >>> - } \ >>> + >>> +static void s390_pv_cmd_exit(uint32_t cmd, void *data) >>> +{ >>> + if (s390_pv_cmd(cmd, data)) { >>> + exit(1); >>> + } >>> } >>> int s390_pv_query_info(void) >> >> >
diff --git a/target/s390x/kvm/pv.c b/target/s390x/kvm/pv.c index 3a0a971f0b..66194caaae 100644 --- a/target/s390x/kvm/pv.c +++ b/target/s390x/kvm/pv.c @@ -59,14 +59,12 @@ static int __s390_pv_cmd(uint32_t cmd, const char *cmdname, void *data, */ #define s390_pv_cmd(cmd, data) __s390_pv_cmd(cmd, #cmd, data, NULL) #define s390_pv_cmd_pvrc(cmd, data, pvrc) __s390_pv_cmd(cmd, #cmd, data, pvrc) -#define s390_pv_cmd_exit(cmd, data) \ -{ \ - int rc; \ - \ - rc = __s390_pv_cmd(cmd, #cmd, data, NULL); \ - if (rc) { \ - exit(1); \ - } \ + +static void s390_pv_cmd_exit(uint32_t cmd, void *data) +{ + if (s390_pv_cmd(cmd, data)) { + exit(1); + } } int s390_pv_query_info(void)
introduce a static function when exiting PV. The function replaces an existing macro (s390_pv_cmd_exit). Signed-off-by: Gautam Gala <ggala@linux.ibm.com> --- target/s390x/kvm/pv.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)