| Message ID | 20200404131017.27330-7-julien@xen.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen: Consolidate asm-*/guest_access.h in xen/guest_access.h | expand |
On 04.04.2020 15:10, Julien Grall wrote: > From: Julien Grall <jgrall@amazon.com> > > Most of the helpers to access guest memory are implemented the same way > on Arm and x86. The only differences are: > - guest_handle_{from, to}_param(): while on x86 XEN_GUEST_HANDLE() > and XEN_GUEST_HANDLE_PARAM() are the same, they are not on Arm. It > is still fine to use the Arm implementation on x86. > - __clear_guest_offset(): Interestingly the prototype does not match > between the x86 and Arm. However, the Arm one is bogus. So the x86 > implementation can be used. > - guest_handle{,_subrange}_okay(): They are validly differing > because Arm is only supporting auto-translated guest and therefore > handles are always valid. While I'm fine in principle with such consolidation, I'm afraid I really need to ask for some historical background to be added here. It may very well be that there's a reason for the separation (likely to be found in the removed ia64 or ppc ports), which may then provide a hint at why future ports may want to have these separated. If such reasons exist, I'd prefer to avoid the back and forth between headers. What we could do in such a case is borrow Linux'es asm-generic/ concept, and move the "typical" implementation there. (And of course if there were no noticable reasons for the split, the change as it is would be fine in general; saying so without having looked at the details of it, yet). Jan
Hi Jan, On 07/04/2020 09:14, Jan Beulich wrote: > On 04.04.2020 15:10, Julien Grall wrote: >> From: Julien Grall <jgrall@amazon.com> >> >> Most of the helpers to access guest memory are implemented the same way >> on Arm and x86. The only differences are: >> - guest_handle_{from, to}_param(): while on x86 XEN_GUEST_HANDLE() >> and XEN_GUEST_HANDLE_PARAM() are the same, they are not on Arm. It >> is still fine to use the Arm implementation on x86. >> - __clear_guest_offset(): Interestingly the prototype does not match >> between the x86 and Arm. However, the Arm one is bogus. So the x86 >> implementation can be used. >> - guest_handle{,_subrange}_okay(): They are validly differing >> because Arm is only supporting auto-translated guest and therefore >> handles are always valid. > > While I'm fine in principle with such consolidation, I'm afraid I > really need to ask for some historical background to be added > here. It may very well be that there's a reason for the separation > (likely to be found in the removed ia64 or ppc ports), which may > then provide a hint at why future ports may want to have these > separated. If such reasons exist, I'd prefer to avoid the back and > forth between headers. What we could do in such a case is borrow > Linux'es asm-generic/ concept, and move the "typical" > implementation there. (And of course if there were no noticable > reasons for the split, the change as it is would be fine in > general; saying so without having looked at the details of it, > yet). Looking at the history, ia64 and ppc used to include a common header called xen/xencomm.h from asm/guest_access.h. This has now disappeared with the removal of the two ports. Regarding future arch, the fact arm and x86 gives me some confidence we are unlikely going to get a new ABI for an arch. Do you see any reason to? Cheers,
On 09.04.2020 00:05, Julien Grall wrote: > Hi Jan, > > On 07/04/2020 09:14, Jan Beulich wrote: >> On 04.04.2020 15:10, Julien Grall wrote: >>> From: Julien Grall <jgrall@amazon.com> >>> >>> Most of the helpers to access guest memory are implemented the same way >>> on Arm and x86. The only differences are: >>> - guest_handle_{from, to}_param(): while on x86 XEN_GUEST_HANDLE() >>> and XEN_GUEST_HANDLE_PARAM() are the same, they are not on Arm. It >>> is still fine to use the Arm implementation on x86. >>> - __clear_guest_offset(): Interestingly the prototype does not match >>> between the x86 and Arm. However, the Arm one is bogus. So the x86 >>> implementation can be used. >>> - guest_handle{,_subrange}_okay(): They are validly differing >>> because Arm is only supporting auto-translated guest and therefore >>> handles are always valid. >> >> While I'm fine in principle with such consolidation, I'm afraid I >> really need to ask for some historical background to be added >> here. It may very well be that there's a reason for the separation >> (likely to be found in the removed ia64 or ppc ports), which may >> then provide a hint at why future ports may want to have these >> separated. If such reasons exist, I'd prefer to avoid the back and >> forth between headers. What we could do in such a case is borrow >> Linux'es asm-generic/ concept, and move the "typical" >> implementation there. (And of course if there were no noticable >> reasons for the split, the change as it is would be fine in >> general; saying so without having looked at the details of it, >> yet). > > Looking at the history, ia64 and ppc used to include a common > header called xen/xencomm.h from asm/guest_access.h. > > This has now disappeared with the removal of the two ports. > > Regarding future arch, the fact arm and x86 gives me some confidence > we are unlikely going to get a new ABI for an arch. Do you see any > reason to? Well, there surely had be a reason for ia64 and ppc to use a different approach. I don't see why a new port may not also want to go that route instead of the x86/Arm one. Jan
Hi, On 09/04/2020 07:30, Jan Beulich wrote: > On 09.04.2020 00:05, Julien Grall wrote: >> Hi Jan, >> >> On 07/04/2020 09:14, Jan Beulich wrote: >>> On 04.04.2020 15:10, Julien Grall wrote: >>>> From: Julien Grall <jgrall@amazon.com> >>>> >>>> Most of the helpers to access guest memory are implemented the same way >>>> on Arm and x86. The only differences are: >>>> - guest_handle_{from, to}_param(): while on x86 XEN_GUEST_HANDLE() >>>> and XEN_GUEST_HANDLE_PARAM() are the same, they are not on Arm. It >>>> is still fine to use the Arm implementation on x86. >>>> - __clear_guest_offset(): Interestingly the prototype does not match >>>> between the x86 and Arm. However, the Arm one is bogus. So the x86 >>>> implementation can be used. >>>> - guest_handle{,_subrange}_okay(): They are validly differing >>>> because Arm is only supporting auto-translated guest and therefore >>>> handles are always valid. >>> >>> While I'm fine in principle with such consolidation, I'm afraid I >>> really need to ask for some historical background to be added >>> here. It may very well be that there's a reason for the separation >>> (likely to be found in the removed ia64 or ppc ports), which may >>> then provide a hint at why future ports may want to have these >>> separated. If such reasons exist, I'd prefer to avoid the back and >>> forth between headers. What we could do in such a case is borrow >>> Linux'es asm-generic/ concept, and move the "typical" >>> implementation there. (And of course if there were no noticable >>> reasons for the split, the change as it is would be fine in >>> general; saying so without having looked at the details of it, >>> yet). >> >> Looking at the history, ia64 and ppc used to include a common >> header called xen/xencomm.h from asm/guest_access.h. >> >> This has now disappeared with the removal of the two ports. >> >> Regarding future arch, the fact arm and x86 gives me some confidence >> we are unlikely going to get a new ABI for an arch. Do you see any >> reason to? > > Well, there surely had be a reason for ia64 and ppc to use a > different approach. This was introduced way before my time in Xen. AFAICT, you were already part of the community when 'xencomm' were alive. There are not much information about it only nor in the commits. So do you mind sharing a bit more what 'xencomm' meant to be and why it wasn't introduced on x86? > I don't see why a new port may not also want > to go that route instead of the x86/Arm one. I could accept that someone would want to reinvent a new ABI from scratch for just an hypothetical new arch. However it would be quite an effort to reinvent XEN_GUEST_HANDLE(). The chance is RISC-V is only going to re-use what Arm did as Arm already did with x86. I would like to avoid to introduce a new directory asm-generic with just one header in it. Maybe you have some other headers in mind? Cheers,
On 09.04.2020 10:01, Julien Grall wrote: > Hi, > > On 09/04/2020 07:30, Jan Beulich wrote: >> On 09.04.2020 00:05, Julien Grall wrote: >>> Hi Jan, >>> >>> On 07/04/2020 09:14, Jan Beulich wrote: >>>> On 04.04.2020 15:10, Julien Grall wrote: >>>>> From: Julien Grall <jgrall@amazon.com> >>>>> >>>>> Most of the helpers to access guest memory are implemented the same way >>>>> on Arm and x86. The only differences are: >>>>> - guest_handle_{from, to}_param(): while on x86 XEN_GUEST_HANDLE() >>>>> and XEN_GUEST_HANDLE_PARAM() are the same, they are not on Arm. It >>>>> is still fine to use the Arm implementation on x86. >>>>> - __clear_guest_offset(): Interestingly the prototype does not match >>>>> between the x86 and Arm. However, the Arm one is bogus. So the x86 >>>>> implementation can be used. >>>>> - guest_handle{,_subrange}_okay(): They are validly differing >>>>> because Arm is only supporting auto-translated guest and therefore >>>>> handles are always valid. >>>> >>>> While I'm fine in principle with such consolidation, I'm afraid I >>>> really need to ask for some historical background to be added >>>> here. It may very well be that there's a reason for the separation >>>> (likely to be found in the removed ia64 or ppc ports), which may >>>> then provide a hint at why future ports may want to have these >>>> separated. If such reasons exist, I'd prefer to avoid the back and >>>> forth between headers. What we could do in such a case is borrow >>>> Linux'es asm-generic/ concept, and move the "typical" >>>> implementation there. (And of course if there were no noticable >>>> reasons for the split, the change as it is would be fine in >>>> general; saying so without having looked at the details of it, >>>> yet). >>> >>> Looking at the history, ia64 and ppc used to include a common >>> header called xen/xencomm.h from asm/guest_access.h. >>> >>> This has now disappeared with the removal of the two ports. >>> >>> Regarding future arch, the fact arm and x86 gives me some confidence >>> we are unlikely going to get a new ABI for an arch. Do you see any >>> reason to? >> >> Well, there surely had be a reason for ia64 and ppc to use a >> different approach. > > This was introduced way before my time in Xen. AFAICT, you were > already part of the community when 'xencomm' were alive. I was, but I was never actively involved in ia64 or ppc work. > There are not much information about it only nor in the commits. > So do you mind sharing a bit more what 'xencomm' meant to be > and why it wasn't introduced on x86? If I had details, I would have provided at least some hints already. >> I don't see why a new port may not also want >> to go that route instead of the x86/Arm one. > I could accept that someone would want to reinvent a new ABI > from scratch for just an hypothetical new arch. However it would > be quite an effort to reinvent XEN_GUEST_HANDLE(). The chance is > RISC-V is only going to re-use what Arm did as Arm already did > with x86. > > I would like to avoid to introduce a new directory asm-generic > with just one header in it. Maybe you have some other headers in > mind? I recall having wondered a few times whether we shouldn't use this concept elsewhere. One case iirc was bitops stuff. Looking over the Linux ones, some atomic and barrier fallback implementations may also sensibly live there, and there are likely more. Jan
On 09/04/2020 09:06, Jan Beulich wrote: > On 09.04.2020 10:01, Julien Grall wrote: >> Hi, >> >> On 09/04/2020 07:30, Jan Beulich wrote: >>> On 09.04.2020 00:05, Julien Grall wrote: >>> I don't see why a new port may not also want >>> to go that route instead of the x86/Arm one. >> I could accept that someone would want to reinvent a new ABI >> from scratch for just an hypothetical new arch. However it would >> be quite an effort to reinvent XEN_GUEST_HANDLE(). The chance is >> RISC-V is only going to re-use what Arm did as Arm already did >> with x86. >> >> I would like to avoid to introduce a new directory asm-generic >> with just one header in it. Maybe you have some other headers in >> mind? > > I recall having wondered a few times whether we shouldn't use this > concept elsewhere. One case iirc was bitops stuff. Looking over > the Linux ones, some atomic and barrier fallback implementations > may also sensibly live there, and there are likely more. In theory it makes sense but, in the current state of Xen, 'asm-generic' means they are common to Arm and x86. This is basically the same definition as for the directory 'xen'. So how do you draw a line which files go where? To be honest, I don't think we can draw a line without a 3rd architecture. So I would recommend to wait until then to create an asm-generic directory. Meanwhile, I still think the consolidation in xen/ is useful as it makes easier to maintain. It is also going to make easier for RISCv (or a new arch) as they don't have to worry about duplication (if any). In the event they decide to purse their own route, then it is not going to be a massive pain to move part of xen/guest_access.h in asm-generic/guest_access.h and include the latter from {xen, asm} /guest_access.h. Cheers,
Hi, Gentle ping. Any comments on the direction to take? On 09/04/2020 10:28, Julien Grall wrote: > > > On 09/04/2020 09:06, Jan Beulich wrote: >> On 09.04.2020 10:01, Julien Grall wrote: >>> Hi, >>> >>> On 09/04/2020 07:30, Jan Beulich wrote: >>>> On 09.04.2020 00:05, Julien Grall wrote: >>>> I don't see why a new port may not also want >>>> to go that route instead of the x86/Arm one. >>> I could accept that someone would want to reinvent a new ABI >>> from scratch for just an hypothetical new arch. However it would >>> be quite an effort to reinvent XEN_GUEST_HANDLE(). The chance is >>> RISC-V is only going to re-use what Arm did as Arm already did >>> with x86. >>> >>> I would like to avoid to introduce a new directory asm-generic >>> with just one header in it. Maybe you have some other headers in >>> mind? >> >> I recall having wondered a few times whether we shouldn't use this >> concept elsewhere. One case iirc was bitops stuff. Looking over >> the Linux ones, some atomic and barrier fallback implementations >> may also sensibly live there, and there are likely more. > > In theory it makes sense but, in the current state of Xen, 'asm-generic' > means they are common to Arm and x86. This is basically the same > definition as for the directory 'xen'. So how do you draw a line which > files go where? > > To be honest, I don't think we can draw a line without a 3rd > architecture. So I would recommend to wait until then to create an > asm-generic directory. > > Meanwhile, I still think the consolidation in xen/ is useful as it makes > easier to maintain. It is also going to make easier for RISCv (or a new > arch) as they don't have to worry about duplication (if any). > > In the event they decide to purse their own route, then it is not going > to be a massive pain to move part of xen/guest_access.h in > asm-generic/guest_access.h and include the latter from {xen, asm} > /guest_access.h. Cheers,
On 29.04.2020 16:04, Julien Grall wrote: > Gentle ping. Any comments on the direction to take? Just to avoid misunderstanding - while the mail was sent with me as the only one on the To list, I don't think I've been meant, as I've voiced my opinion. I assume you rather meant to ping others to chime in? Jan > On 09/04/2020 10:28, Julien Grall wrote: >> >> >> On 09/04/2020 09:06, Jan Beulich wrote: >>> On 09.04.2020 10:01, Julien Grall wrote: >>>> Hi, >>>> >>>> On 09/04/2020 07:30, Jan Beulich wrote: >>>>> On 09.04.2020 00:05, Julien Grall wrote: >>>>> I don't see why a new port may not also want >>>>> to go that route instead of the x86/Arm one. >>>> I could accept that someone would want to reinvent a new ABI >>>> from scratch for just an hypothetical new arch. However it would >>>> be quite an effort to reinvent XEN_GUEST_HANDLE(). The chance is >>>> RISC-V is only going to re-use what Arm did as Arm already did >>>> with x86. >>>> >>>> I would like to avoid to introduce a new directory asm-generic >>>> with just one header in it. Maybe you have some other headers in >>>> mind? >>> >>> I recall having wondered a few times whether we shouldn't use this >>> concept elsewhere. One case iirc was bitops stuff. Looking over >>> the Linux ones, some atomic and barrier fallback implementations >>> may also sensibly live there, and there are likely more. >> >> In theory it makes sense but, in the current state of Xen, 'asm-generic' means they are common to Arm and x86. This is basically the same definition as for the directory 'xen'. So how do you draw a line which files go where? >> >> To be honest, I don't think we can draw a line without a 3rd architecture. So I would recommend to wait until then to create an asm-generic directory. >> >> Meanwhile, I still think the consolidation in xen/ is useful as it makes easier to maintain. It is also going to make easier for RISCv (or a new arch) as they don't have to worry about duplication (if any). >> >> In the event they decide to purse their own route, then it is not going to be a massive pain to move part of xen/guest_access.h in asm-generic/guest_access.h and include the latter from {xen, asm} /guest_access.h. > > Cheers, >
Hi, On 29/04/2020 15:07, Jan Beulich wrote: > On 29.04.2020 16:04, Julien Grall wrote: >> Gentle ping. Any comments on the direction to take? > > Just to avoid misunderstanding - while the mail was sent with me as > the only one on the To list, I don't think I've been meant, as I've > voiced my opinion. I assume you rather meant to ping others to chime > in? I barely pay attention to CC vs TO. If I am on the list of recipients of an e-mail, then I will at least have a glance. This e-mail is directed to you specifically and also the others. While you may have voiced some of your opinion already, you haven't replied back how you would decide whether an header should be added in xen or asm-generic. So can you please have another and explain how the line can be drawn with just two architectures in place. Cheers,
On 29.04.2020 16:13, Julien Grall wrote:
> So can you please have another and explain how the line can be drawn with just two architectures in place.
There are abstract considerations that can be used to draw the
line, as well as knowledge of people on architectures Xen doesn't
run on, but where one can - with such knowledge - extrapolate how
it would want to be implemented.
I don't think the question at this point is where to draw the
line, but whether to have asm-generic/ in the first place.
Jan
On 29/04/2020 15:54, Jan Beulich wrote: > On 29.04.2020 16:13, Julien Grall wrote: >> So can you please have another and explain how the line can be drawn with just two architectures in place. > > There are abstract considerations that can be used to draw the > line, as well as knowledge of people on architectures Xen doesn't > run on, but where one can - with such knowledge - extrapolate how > it would want to be implemented. > > I don't think the question at this point is where to draw the > line, but whether to have asm-generic/ in the first place. Well the two come together. You can't add a new directory with no clear view how this is going to be used. At the moment, this would result at best bikeshedding because developpers may have a different opinion on how a new architecture would be implemented in Xen. If you have a 3rd architectures then it would be easier to argue the header should be added in asm-generic/ or xen/: - asm-generic/ should be used if 2 of the architectures are using the same interface - xen/ should be if the 3 architectures are using the same interface Cheers,
Ping 2. On 29/04/2020 15:04, Julien Grall wrote: > Hi, > > Gentle ping. Any comments on the direction to take? > > On 09/04/2020 10:28, Julien Grall wrote: >> >> >> On 09/04/2020 09:06, Jan Beulich wrote: >>> On 09.04.2020 10:01, Julien Grall wrote: >>>> Hi, >>>> >>>> On 09/04/2020 07:30, Jan Beulich wrote: >>>>> On 09.04.2020 00:05, Julien Grall wrote: >>>>> I don't see why a new port may not also want >>>>> to go that route instead of the x86/Arm one. >>>> I could accept that someone would want to reinvent a new ABI >>>> from scratch for just an hypothetical new arch. However it would >>>> be quite an effort to reinvent XEN_GUEST_HANDLE(). The chance is >>>> RISC-V is only going to re-use what Arm did as Arm already did >>>> with x86. >>>> >>>> I would like to avoid to introduce a new directory asm-generic >>>> with just one header in it. Maybe you have some other headers in >>>> mind? >>> >>> I recall having wondered a few times whether we shouldn't use this >>> concept elsewhere. One case iirc was bitops stuff. Looking over >>> the Linux ones, some atomic and barrier fallback implementations >>> may also sensibly live there, and there are likely more. >> >> In theory it makes sense but, in the current state of Xen, >> 'asm-generic' means they are common to Arm and x86. This is basically >> the same definition as for the directory 'xen'. So how do you draw a >> line which files go where? >> >> To be honest, I don't think we can draw a line without a 3rd >> architecture. So I would recommend to wait until then to create an >> asm-generic directory. >> >> Meanwhile, I still think the consolidation in xen/ is useful as it >> makes easier to maintain. It is also going to make easier for RISCv >> (or a new arch) as they don't have to worry about duplication (if any). >> >> In the event they decide to purse their own route, then it is not >> going to be a massive pain to move part of xen/guest_access.h in >> asm-generic/guest_access.h and include the latter from {xen, asm} >> /guest_access.h. > > Cheers, >
Hi. My attention was drawn to this thread. As I understand it, everyone is agreed that deduplicating the implementation is good (I also agree). The debate is only between: 1. Put it in xen/ until an arch comes along that needs something different, at which point maybe introduce an asm-generic-style thing with default implementations. 2. Say, now, that this is a default implementation and it should go in asm-generic. My starting point is that Julien, as the primary author of this cleanup, should be given leeway on a matter of taste like this. (There are as I understand it no wider implications.) Also, ISTM that it can be argued that introducing a new abstraction is an additional piece of work. Doing that is certainly not hampered by Julien's change. So that would be another reason to take Julien's patch as-is. On the merits, I don't have anything to add to the arguments already presented. I am considerably more persuaded by Julien's arguments than Jan's. So on all levels I think this commit should go in, unless there are other concerns that have not been discussed here ? Thanks, Ian.
Hi Ian, On 19/05/2020 16:05, Ian Jackson wrote: > Hi. My attention was drawn to this thread. > > As I understand it, everyone is agreed that deduplicating the > implementation is good (I also agree). The debate is only between: Thank you for stepping in! > > 1. Put it in xen/ until an arch comes along that needs something > different, at which point maybe introduce an asm-generic-style > thing with default implementations. > > 2. Say, now, that this is a default implementation and it should go in > asm-generic. > > My starting point is that Julien, as the primary author of this > cleanup, should be given leeway on a matter of taste like this. > (There are as I understand it no wider implications.) > > Also, ISTM that it can be argued that introducing a new abstraction is > an additional piece of work. Doing that is certainly not hampered by > Julien's change. So that would be another reason to take Julien's > patch as-is. > > On the merits, I don't have anything to add to the arguments already > presented. I am considerably more persuaded by Julien's arguments > than Jan's. > > So on all levels I think this commit should go in, unless there are > other concerns that have not been discussed here ? The major blocker is where the common header lives. The rest are small comments I should address in the next version. I will send a new version (probably post freeze) to address those comments. Cheers,
diff --git a/xen/include/asm-arm/guest_access.h b/xen/include/asm-arm/guest_access.h index 93d56868f1..53766386d3 100644 --- a/xen/include/asm-arm/guest_access.h +++ b/xen/include/asm-arm/guest_access.h @@ -23,102 +23,6 @@ int access_guest_memory_by_ipa(struct domain *d, paddr_t ipa, void *buf, #define __raw_copy_from_guest raw_copy_from_guest #define __raw_clear_guest raw_clear_guest -/* Remainder copied from x86 -- could be common? */ - -/* Is the guest handle a NULL reference? */ -#define guest_handle_is_null(hnd) ((hnd).p == NULL) - -/* Offset the given guest handle into the array it refers to. */ -#define guest_handle_add_offset(hnd, nr) ((hnd).p += (nr)) -#define guest_handle_subtract_offset(hnd, nr) ((hnd).p -= (nr)) - -/* Cast a guest handle (either XEN_GUEST_HANDLE or XEN_GUEST_HANDLE_PARAM) - * to the specified type of XEN_GUEST_HANDLE_PARAM. */ -#define guest_handle_cast(hnd, type) ({ \ - type *_x = (hnd).p; \ - (XEN_GUEST_HANDLE_PARAM(type)) { _x }; \ -}) - -/* Cast a XEN_GUEST_HANDLE to XEN_GUEST_HANDLE_PARAM */ -#define guest_handle_to_param(hnd, type) ({ \ - typeof((hnd).p) _x = (hnd).p; \ - XEN_GUEST_HANDLE_PARAM(type) _y = { _x }; \ - /* type checking: make sure that the pointers inside \ - * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of \ - * the same type, then return hnd */ \ - (void)(&_x == &_y.p); \ - _y; \ -}) - - -/* Cast a XEN_GUEST_HANDLE_PARAM to XEN_GUEST_HANDLE */ -#define guest_handle_from_param(hnd, type) ({ \ - typeof((hnd).p) _x = (hnd).p; \ - XEN_GUEST_HANDLE(type) _y = { _x }; \ - /* type checking: make sure that the pointers inside \ - * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of \ - * the same type, then return hnd */ \ - (void)(&_x == &_y.p); \ - _y; \ -}) - -#define guest_handle_for_field(hnd, type, fld) \ - ((XEN_GUEST_HANDLE(type)) { &(hnd).p->fld }) - -#define guest_handle_from_ptr(ptr, type) \ - ((XEN_GUEST_HANDLE_PARAM(type)) { (type *)ptr }) -#define const_guest_handle_from_ptr(ptr, type) \ - ((XEN_GUEST_HANDLE_PARAM(const_##type)) { (const type *)ptr }) - -/* - * Copy an array of objects to guest context via a guest handle, - * specifying an offset into the guest array. - * - * The variable _t is only here to catch at build time whether we are - * copying back to a const guest handle. - */ -#define copy_to_guest_offset(hnd, off, ptr, nr) ({ \ - const typeof(*(ptr)) *_s = (ptr); \ - char (*_d)[sizeof(*_s)] = (void *)(hnd).p; \ - void *__maybe_unused _t = (hnd).p; \ - ((void)((hnd).p == (ptr))); \ - raw_copy_to_guest(_d+(off), _s, sizeof(*_s)*(nr)); \ -}) - -/* - * Clear an array of objects in guest context via a guest handle, - * specifying an offset into the guest array. - */ -#define clear_guest_offset(hnd, off, nr) ({ \ - void *_d = (hnd).p; \ - raw_clear_guest(_d+(off), nr); \ -}) - -/* - * Copy an array of objects from guest context via a guest handle, - * specifying an offset into the guest array. - */ -#define copy_from_guest_offset(ptr, hnd, off, nr) ({ \ - const typeof(*(ptr)) *_s = (hnd).p; \ - typeof(*(ptr)) *_d = (ptr); \ - raw_copy_from_guest(_d, _s+(off), sizeof(*_d)*(nr));\ -}) - -/* Copy sub-field of a structure to guest context via a guest handle. */ -#define copy_field_to_guest(hnd, ptr, field) ({ \ - const typeof(&(ptr)->field) _s = &(ptr)->field; \ - void *_d = &(hnd).p->field; \ - ((void)(&(hnd).p->field == &(ptr)->field)); \ - raw_copy_to_guest(_d, _s, sizeof(*_s)); \ -}) - -/* Copy sub-field of a structure from guest context via a guest handle. */ -#define copy_field_from_guest(ptr, hnd, field) ({ \ - const typeof(&(ptr)->field) _s = &(hnd).p->field; \ - typeof(&(ptr)->field) _d = &(ptr)->field; \ - raw_copy_from_guest(_d, _s, sizeof(*_d)); \ -}) - /* * Pre-validate a guest handle. * Allows use of faster __copy_* functions. @@ -127,41 +31,6 @@ int access_guest_memory_by_ipa(struct domain *d, paddr_t ipa, void *buf, #define guest_handle_okay(hnd, nr) (1) #define guest_handle_subrange_okay(hnd, first, last) (1) -/* - * The variable _t is only here to catch at build time whether we are - * copying back to a const guest handle. - */ -#define __copy_to_guest_offset(hnd, off, ptr, nr) ({ \ - const typeof(*(ptr)) *_s = (ptr); \ - char (*_d)[sizeof(*_s)] = (void *)(hnd).p; \ - void *__maybe_unused _t = (hnd).p; \ - ((void)((hnd).p == (ptr))); \ - __raw_copy_to_guest(_d+(off), _s, sizeof(*_s)*(nr));\ -}) - -#define __clear_guest_offset(hnd, off, ptr, nr) ({ \ - __raw_clear_guest(_d+(off), nr); \ -}) - -#define __copy_from_guest_offset(ptr, hnd, off, nr) ({ \ - const typeof(*(ptr)) *_s = (hnd).p; \ - typeof(*(ptr)) *_d = (ptr); \ - __raw_copy_from_guest(_d, _s+(off), sizeof(*_d)*(nr));\ -}) - -#define __copy_field_to_guest(hnd, ptr, field) ({ \ - const typeof(&(ptr)->field) _s = &(ptr)->field; \ - void *_d = &(hnd).p->field; \ - ((void)(&(hnd).p->field == &(ptr)->field)); \ - __raw_copy_to_guest(_d, _s, sizeof(*_s)); \ -}) - -#define __copy_field_from_guest(ptr, hnd, field) ({ \ - const typeof(&(ptr)->field) _s = &(hnd).p->field; \ - typeof(&(ptr)->field) _d = &(ptr)->field; \ - __raw_copy_from_guest(_d, _s, sizeof(*_d)); \ -}) - #endif /* __ASM_ARM_GUEST_ACCESS_H__ */ /* * Local variables: diff --git a/xen/include/asm-x86/guest_access.h b/xen/include/asm-x86/guest_access.h index 5c3dfc47b6..08c9fbbc78 100644 --- a/xen/include/asm-x86/guest_access.h +++ b/xen/include/asm-x86/guest_access.h @@ -38,95 +38,6 @@ clear_user_hvm((dst), (len)) : \ clear_user((dst), (len))) -/* Is the guest handle a NULL reference? */ -#define guest_handle_is_null(hnd) ((hnd).p == NULL) - -/* Offset the given guest handle into the array it refers to. */ -#define guest_handle_add_offset(hnd, nr) ((hnd).p += (nr)) -#define guest_handle_subtract_offset(hnd, nr) ((hnd).p -= (nr)) - -/* Cast a guest handle (either XEN_GUEST_HANDLE or XEN_GUEST_HANDLE_PARAM) - * to the specified type of XEN_GUEST_HANDLE_PARAM. */ -#define guest_handle_cast(hnd, type) ({ \ - type *_x = (hnd).p; \ - (XEN_GUEST_HANDLE_PARAM(type)) { _x }; \ -}) - -/* Cast a XEN_GUEST_HANDLE to XEN_GUEST_HANDLE_PARAM */ -#define guest_handle_to_param(hnd, type) ({ \ - typeof((hnd).p) _x = (hnd).p; \ - XEN_GUEST_HANDLE_PARAM(type) _y = { _x }; \ - /* type checking: make sure that the pointers inside \ - * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of \ - * the same type, then return hnd */ \ - (void)(&_x == &_y.p); \ - _y; \ -}) - -/* Cast a XEN_GUEST_HANDLE_PARAM to XEN_GUEST_HANDLE */ -#define guest_handle_from_param(hnd, type) ({ \ - typeof((hnd).p) _x = (hnd).p; \ - XEN_GUEST_HANDLE(type) _y = { _x }; \ - /* type checking: make sure that the pointers inside \ - * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of \ - * the same type, then return hnd */ \ - (void)(&_x == &_y.p); \ - _y; \ -}) - -#define guest_handle_for_field(hnd, type, fld) \ - ((XEN_GUEST_HANDLE(type)) { &(hnd).p->fld }) - -#define guest_handle_from_ptr(ptr, type) \ - ((XEN_GUEST_HANDLE_PARAM(type)) { (type *)ptr }) -#define const_guest_handle_from_ptr(ptr, type) \ - ((XEN_GUEST_HANDLE_PARAM(const_##type)) { (const type *)ptr }) - -/* - * Copy an array of objects to guest context via a guest handle, - * specifying an offset into the guest array. - * - * The variable _t is only here to catch at build time whether we are - * copying back to a const guest handle. - */ -#define copy_to_guest_offset(hnd, off, ptr, nr) ({ \ - const typeof(*(ptr)) *_s = (ptr); \ - char (*_d)[sizeof(*_s)] = (void *)(hnd).p; \ - void *__maybe_unused _t = (hnd).p; \ - ((void)((hnd).p == (ptr))); \ - raw_copy_to_guest(_d+(off), _s, sizeof(*_s)*(nr)); \ -}) - -/* - * Copy an array of objects from guest context via a guest handle, - * specifying an offset into the guest array. - */ -#define copy_from_guest_offset(ptr, hnd, off, nr) ({ \ - const typeof(*(ptr)) *_s = (hnd).p; \ - typeof(*(ptr)) *_d = (ptr); \ - raw_copy_from_guest(_d, _s+(off), sizeof(*_d)*(nr));\ -}) - -#define clear_guest_offset(hnd, off, nr) ({ \ - void *_d = (hnd).p; \ - raw_clear_guest(_d+(off), nr); \ -}) - -/* Copy sub-field of a structure to guest context via a guest handle. */ -#define copy_field_to_guest(hnd, ptr, field) ({ \ - const typeof(&(ptr)->field) _s = &(ptr)->field; \ - void *_d = &(hnd).p->field; \ - ((void)(&(hnd).p->field == &(ptr)->field)); \ - raw_copy_to_guest(_d, _s, sizeof(*_s)); \ -}) - -/* Copy sub-field of a structure from guest context via a guest handle. */ -#define copy_field_from_guest(ptr, hnd, field) ({ \ - const typeof(&(ptr)->field) _s = &(hnd).p->field; \ - typeof(&(ptr)->field) _d = &(ptr)->field; \ - raw_copy_from_guest(_d, _s, sizeof(*_d)); \ -}) - /* * Pre-validate a guest handle. * Allows use of faster __copy_* functions. @@ -140,42 +51,6 @@ (last)-(first)+1, \ sizeof(*(hnd).p))) -/* - * The variable _t is only here to catch at build time whether we are - * copying back to a const guest handle. - */ -#define __copy_to_guest_offset(hnd, off, ptr, nr) ({ \ - const typeof(*(ptr)) *_s = (ptr); \ - char (*_d)[sizeof(*_s)] = (void *)(hnd).p; \ - void *__maybe_unused _t = (hnd).p; \ - ((void)((hnd).p == (ptr))); \ - __raw_copy_to_guest(_d+(off), _s, sizeof(*_s)*(nr));\ -}) - -#define __copy_from_guest_offset(ptr, hnd, off, nr) ({ \ - const typeof(*(ptr)) *_s = (hnd).p; \ - typeof(*(ptr)) *_d = (ptr); \ - __raw_copy_from_guest(_d, _s+(off), sizeof(*_d)*(nr));\ -}) - -#define __clear_guest_offset(hnd, off, nr) ({ \ - void *_d = (hnd).p; \ - __raw_clear_guest(_d+(off), nr); \ -}) - -#define __copy_field_to_guest(hnd, ptr, field) ({ \ - const typeof(&(ptr)->field) _s = &(ptr)->field; \ - void *_d = &(hnd).p->field; \ - ((void)(&(hnd).p->field == &(ptr)->field)); \ - __raw_copy_to_guest(_d, _s, sizeof(*_s)); \ -}) - -#define __copy_field_from_guest(ptr, hnd, field) ({ \ - const typeof(&(ptr)->field) _s = &(hnd).p->field; \ - typeof(&(ptr)->field) _d = &(ptr)->field; \ - __raw_copy_from_guest(_d, _s, sizeof(*_d)); \ -}) - #endif /* __ASM_X86_GUEST_ACCESS_H__ */ /* * Local variables: diff --git a/xen/include/xen/guest_access.h b/xen/include/xen/guest_access.h index ef9aaa3efc..f724f995c3 100644 --- a/xen/include/xen/guest_access.h +++ b/xen/include/xen/guest_access.h @@ -11,6 +11,99 @@ #include <xen/types.h> #include <public/xen.h> +/* Is the guest handle a NULL reference? */ +#define guest_handle_is_null(hnd) ((hnd).p == NULL) + +/* Offset the given guest handle into the array it refers to. */ +#define guest_handle_add_offset(hnd, nr) ((hnd).p += (nr)) +#define guest_handle_subtract_offset(hnd, nr) ((hnd).p -= (nr)) + +/* Cast a guest handle (either XEN_GUEST_HANDLE or XEN_GUEST_HANDLE_PARAM) + * to the specified type of XEN_GUEST_HANDLE_PARAM. */ +#define guest_handle_cast(hnd, type) ({ \ + type *_x = (hnd).p; \ + (XEN_GUEST_HANDLE_PARAM(type)) { _x }; \ +}) + +/* Cast a XEN_GUEST_HANDLE to XEN_GUEST_HANDLE_PARAM */ +#define guest_handle_to_param(hnd, type) ({ \ + typeof((hnd).p) _x = (hnd).p; \ + XEN_GUEST_HANDLE_PARAM(type) _y = { _x }; \ + /* type checking: make sure that the pointers inside \ + * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of \ + * the same type, then return hnd */ \ + (void)(&_x == &_y.p); \ + _y; \ +}) + +/* Cast a XEN_GUEST_HANDLE_PARAM to XEN_GUEST_HANDLE */ +#define guest_handle_from_param(hnd, type) ({ \ + typeof((hnd).p) _x = (hnd).p; \ + XEN_GUEST_HANDLE(type) _y = { _x }; \ + /* type checking: make sure that the pointers inside \ + * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of \ + * the same type, then return hnd */ \ + (void)(&_x == &_y.p); \ + _y; \ +}) + +#define guest_handle_for_field(hnd, type, fld) \ + ((XEN_GUEST_HANDLE(type)) { &(hnd).p->fld }) + +#define guest_handle_from_ptr(ptr, type) \ + ((XEN_GUEST_HANDLE_PARAM(type)) { (type *)ptr }) +#define const_guest_handle_from_ptr(ptr, type) \ + ((XEN_GUEST_HANDLE_PARAM(const_##type)) { (const type *)ptr }) + +/* + * Copy an array of objects to guest context via a guest handle, + * specifying an offset into the guest array. + * + * The variable _t is only here to catch at build time whether we are + * copying back to a const guest handle. + */ +#define copy_to_guest_offset(hnd, off, ptr, nr) ({ \ + const typeof(*(ptr)) *_s = (ptr); \ + char (*_d)[sizeof(*_s)] = (void *)(hnd).p; \ + void *__maybe_unused _t = (hnd).p; \ + ((void)((hnd).p == (ptr))); \ + raw_copy_to_guest(_d+(off), _s, sizeof(*_s)*(nr)); \ +}) + +/* + * Clear an array of objects in guest context via a guest handle, + * specifying an offset into the guest array. + */ +#define clear_guest_offset(hnd, off, nr) ({ \ + void *_d = (hnd).p; \ + raw_clear_guest(_d+(off), nr); \ +}) + +/* + * Copy an array of objects from guest context via a guest handle, + * specifying an offset into the guest array. + */ +#define copy_from_guest_offset(ptr, hnd, off, nr) ({ \ + const typeof(*(ptr)) *_s = (hnd).p; \ + typeof(*(ptr)) *_d = (ptr); \ + raw_copy_from_guest(_d, _s+(off), sizeof(*_d)*(nr));\ +}) + +/* Copy sub-field of a structure to guest context via a guest handle. */ +#define copy_field_to_guest(hnd, ptr, field) ({ \ + const typeof(&(ptr)->field) _s = &(ptr)->field; \ + void *_d = &(hnd).p->field; \ + ((void)(&(hnd).p->field == &(ptr)->field)); \ + raw_copy_to_guest(_d, _s, sizeof(*_s)); \ +}) + +/* Copy sub-field of a structure from guest context via a guest handle. */ +#define copy_field_from_guest(ptr, hnd, field) ({ \ + const typeof(&(ptr)->field) _s = &(hnd).p->field; \ + typeof(&(ptr)->field) _d = &(ptr)->field; \ + raw_copy_from_guest(_d, _s, sizeof(*_d)); \ +}) + #define copy_to_guest(hnd, ptr, nr) \ copy_to_guest_offset(hnd, 0, ptr, nr) @@ -20,6 +113,48 @@ #define clear_guest(hnd, nr) \ clear_guest_offset(hnd, 0, nr) +/* + * The __copy_* functions should only be used after the guest handle has + * been pre-validated via guest_handle_okay() and + * guest_handle_subrange_okay(). + */ + +/* + * The variable _t is only here to catch at build time whether we are + * copying back to a const guest handle. + */ +#define __copy_to_guest_offset(hnd, off, ptr, nr) ({ \ + const typeof(*(ptr)) *_s = (ptr); \ + char (*_d)[sizeof(*_s)] = (void *)(hnd).p; \ + void *__maybe_unused _t = (hnd).p; \ + ((void)((hnd).p == (ptr))); \ + __raw_copy_to_guest(_d+(off), _s, sizeof(*_s)*(nr));\ +}) + +#define __clear_guest_offset(hnd, off, nr) ({ \ + void *_d = (hnd).p; \ + __raw_clear_guest(_d + (off), nr); \ +}) + +#define __copy_from_guest_offset(ptr, hnd, off, nr) ({ \ + const typeof(*(ptr)) *_s = (hnd).p; \ + typeof(*(ptr)) *_d = (ptr); \ + __raw_copy_from_guest(_d, _s+(off), sizeof(*_d)*(nr));\ +}) + +#define __copy_field_to_guest(hnd, ptr, field) ({ \ + const typeof(&(ptr)->field) _s = &(ptr)->field; \ + void *_d = &(hnd).p->field; \ + ((void)(&(hnd).p->field == &(ptr)->field)); \ + __raw_copy_to_guest(_d, _s, sizeof(*_s)); \ +}) + +#define __copy_field_from_guest(ptr, hnd, field) ({ \ + const typeof(&(ptr)->field) _s = &(hnd).p->field; \ + typeof(&(ptr)->field) _d = &(ptr)->field; \ + __raw_copy_from_guest(_d, _s, sizeof(*_d)); \ +}) + #define __copy_to_guest(hnd, ptr, nr) \ __copy_to_guest_offset(hnd, 0, ptr, nr)