Re: Weak x86 aliases
> On Dec 28, 2018, at 10:14 AM, Mathew, Cherry G. wrote: > > ic, this probably explains why it's used sparingly so far. Thanks for the > explanation. I'll try to educate myself on this wrt enabling this for modload. We use weak symbols all over the place in user space. It's just that the kernel-resident linker doesn't support them (yet?), so we can't use them for symbols that are part of the ABI exposed to modules. > The other option for me is a bunch of ugly #ifdefs , which I'm trying to > minimise. Reducing #ifdefs is a laudable goal. -- thorpej
Re: Weak x86 aliases
On Sat, Dec 29, 2018 at 01:41:08AM +0530, Mathew, Cherry G. wrote: > Hotpatching will be ugly because we need both versions - for eg: > xen_intr_register_xname() aliases to intr_register_xname() in DomU and > Dom0 but we need both in the case of PVHVM. I don't understand why you need two versions active in one kernel, and also don't see why this would be a problem for runtime patching. Martin
Re: Weak x86 aliases
Le 29/12/2018 à 08:40, Cherry G.Mathew a écrit : Maxime Villard writes: Le 28/12/2018 à 15:06, Cherry G.Mathew a écrit : Maxime Villard writes: Le 28/12/2018 à 14:57, Cherry G.Mathew a écrit : Maxime Villard writes: Introduce a weak alias method of exporting different implementations of the same API. Please revert or fix this change. I'm not sure what the fix is - do you have a suggestion ? either re-apply it without using weak symbols, or change the modloader to accept weak symbols I don't like the imperative in your tone. NVMM is the user of modloader, not PVHVM. So if you feel like your usecase needs fixing, I'd say it's your problem - or don't use modules, but see below. Wut? Yes my suggestions are either we re-apply the change without using weak symbols or we change the modloader to accept weak symbols. Would a __strong_alias() fix this for you ? ie; is aliasing problematic in itself ? Replacing WEAK_ALIAS by STRONG_ALIAS in amd64/amd64/cpufunc.S does fix the problem for me.
Re: Weak x86 aliases
Maxime Villard writes: > Le 28/12/2018 à 15:06, Cherry G.Mathew a écrit : >> Maxime Villard writes: >>> Le 28/12/2018 à 14:57, Cherry G.Mathew a écrit : Maxime Villard writes: >> Introduce a weak alias method of exporting different implementations >> of the same API. > > Please revert or fix this change. I'm not sure what the fix is - do you have a suggestion ? >>> >>> either re-apply it without using weak symbols, or change the modloader >>> to accept weak symbols >> >> I don't like the imperative in your tone. NVMM is the user of modloader, >> not PVHVM. So if you feel like your usecase needs fixing, I'd say it's >> your problem - or don't use modules, but see below. > > Wut? Yes my suggestions are either we re-apply the change without using > weak symbols or we change the modloader to accept weak symbols. > Would a __strong_alias() fix this for you ? ie; is aliasing problematic in itself ? -- ~cherry
Re: Weak x86 aliases
John Nemeth writes: > On Dec 28, 11:52pm, "Mathew, Cherry G." wrote: > } On December 28, 2018 9:54:11 PM GMT+05:30, John Nemeth > wrote: > } >On Dec 28, 7:36pm, "Cherry G.Mathew" wrote: > } >} Maxime Villard writes: > } >} > Le 28/12/2018 �� 14:57, Cherry G.Mathew a ��crit : > } >} >> Maxime Villard writes: > } >} Introduce a weak alias method of exporting different > } >implementations > } >} of the same API. > } >} >>> > } >} >>> Please revert or fix this change. > } >} >> > } >} >> I'm not sure what the fix is - do you have a suggestion ? > } >} > > } >} > either re-apply it without using weak symbols, or change the > } >modloader > } >} > to accept weak symbols > } >} > } >} I don't like the imperative in your tone. NVMM is the user of > } >modloader, > } >} not PVHVM. So if you feel like your usecase needs fixing, I'd say > } >it's > } >} your problem - or don't use modules, but see below. > } > > } > I suspect there's a language issue here due to people using > } >English as a second language. However, I don't see an imperative > } >(command) here. You asked for suggestions on how to fix a problem. > } >He answered your question with a couple of suggestions. That's > } >all. > } > > } > Also, I would argue that the kernel uses modloader, not the > } >module. In any event, as mentioned, it is your change that broke > } >things... > } > } Based on Jason's reply I suspect I've broken modules on Xen too. > } ISTR that you did some work in this area. If you did, can you > } comment? > > I mostly worked on the infrastructure to build a seperate set > of modules for xen and xen-pae. On amd64, Xen modules didn't work > due to missing modmap, which maxv fixed. On i386, only simple > modules worked due to not being able to find symbols. I really > don't know a whole lot about linkers and loaders (manipulating > makefiles is much simpler). > > However, having said that, I suspect that your work with PVHVM > (and presumably PVH after that) will make the idea of having seperate > modules for Xen obsolete. I.e. it appears to me that we're headed > to a world where a single kernel will work both on raw iron and > under Xen. > I would like to maintain PV as is. It has advantages that the PVHVM stuff doesn't have. -- ~cherry
Re: Weak x86 aliases
> On Dec 28, 2018, at 6:44 PM, John Nemeth wrote: > > However, having said that, I suspect that your work with PVHVM > (and presumably PVH after that) will make the idea of having seperate > modules for Xen obsolete. I.e. it appears to me that we're headed > to a world where a single kernel will work both on raw iron and > under Xen. Indeed, Xen x86(-64) modules should be completely compatible with normal x86(-64) modules. If they're not, it means we're leaking too much internal goo into the ABI. -- thorpej
Re: Weak x86 aliases
> On Dec 28, 2018, at 1:32 PM, Paul Goyette wrote: > > The in-kernel linker doesn't deal with weak symbols at all. It would > need a lot of thought to get it right. For example, if module A > (containing a weak reference) gets loaded, its weak references don't > resolve. Then module B gets loaded and defines the symbol(s). Do we > "go back" and re-run the linker for module A? Or do we allow the > results to be different depending on module load order? (If module B > were loaded first, and then module A, the weak reference would get > resolved.) This is why we should't try to support them in the kernel :-) -- thorpej
Re: Weak x86 aliases
On Fri, 28 Dec 2018, Cherry G.Mathew wrote: I think I'll revert these for now, because PVHVM doesn't/shouldn't use them anyway, but I'd like to know how to fix this properly. modload not working due to __weak_alias() sounds like something we don't do properly in the modload path. The in-kernel linker doesn't deal with weak symbols at all. It would need a lot of thought to get it right. For example, if module A (containing a weak reference) gets loaded, its weak references don't resolve. Then module B gets loaded and defines the symbol(s). Do we "go back" and re-run the linker for module A? Or do we allow the results to be different depending on module load order? (If module B were loaded first, and then module A, the weak reference would get resolved.) +--+--++ | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | (Retired)| FA29 0E3B 35AF E8AE 6651 | paul at whooppee dot com | | Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd dot org | +--+--++
Re: Weak x86 aliases
Hotpatching will be ugly because we need both versions - for eg: xen_intr_register_xname() aliases to intr_register_xname() in DomU and Dom0 but we need both in the case of PVHVM. So the other option is function pointers - but this will require re-prototyping all function names that PVHVM touches. I'm open to ideas. -- Sent from my Android device with K-9 Mail. Please excuse my brevity.
Re: Weak x86 aliases
On Fri, Dec 28, 2018 at 07:36:12PM +0530, Cherry G.Mathew wrote: > > There is a check in kobj_sym_lookup(): > > > > 926 case STB_WEAK: > > 927 kobj_error(ko, "weak symbols not supported"); > > 928 return EINVAL; > > > > To resolve correctly I guess we need to take the strongest of the > > duplicated symbols. > > > > I'll look into this unless someone else beats me to it - but for now I'm > ok for the specific change that breaks things for NVMM to be rolled > back. I am not sure I like the idea of weak symbols in the kernel. I would consider either runtime patching (since this is a one time only change at early boot time) or explicitly using function pointers. If you add support for weak symbols to the kobj loader, you will have to also manage re-resolving them after any module unloads, plus finding the ones that have been resolved to symbols provided by the module you are unloading. Martin
Re: Weak x86 aliases
On December 28, 2018 9:54:11 PM GMT+05:30, John Nemeth wrote: >On Dec 28, 7:36pm, "Cherry G.Mathew" wrote: >} Maxime Villard writes: >} > Le 28/12/2018 �� 14:57, Cherry G.Mathew a ��crit : >} >> Maxime Villard writes: >} Introduce a weak alias method of exporting different >implementations >} of the same API. >} >>> >} >>> Please revert or fix this change. >} >> >} >> I'm not sure what the fix is - do you have a suggestion ? >} > >} > either re-apply it without using weak symbols, or change the >modloader >} > to accept weak symbols >} >} I don't like the imperative in your tone. NVMM is the user of >modloader, >} not PVHVM. So if you feel like your usecase needs fixing, I'd say >it's >} your problem - or don't use modules, but see below. > > I suspect there's a language issue here due to people using >English as a second language. However, I don't see an imperative >(command) here. You asked for suggestions on how to fix a problem. >He answered your question with a couple of suggestions. That's >all. > > Also, I would argue that the kernel uses modloader, not the >module. In any event, as mentioned, it is your change that broke >things... > >}-- End of excerpt from "Cherry G.Mathew" Hi John, Based on Jason's reply I suspect I've broken modules on Xen too. ISTR that you did some work in this area. If you did, can you comment? -- Sent from my Android device with K-9 Mail. Please excuse my brevity.
Re: Weak x86 aliases
On December 28, 2018 11:28:01 PM GMT+05:30, Jason Thorpe wrote: > >> On Dec 28, 2018, at 9:50 AM, Mathew, Cherry G. wrote: >> >> That seems like a dependency burden on people developing new >features. I can't tell if modload is the only way to get NVMM to run. >If it is, then ok. If not, then there's the option of provisionally >turning it off until we fix the underlying problem. > >Uh, no :-) This would have been exactly the same situation if you had >changed some other symbol to be a weak-alias. NVMM was using an >exported symbol, no different from how a module might use, say, >kmem_zalloc(), and you changed the semantics of that symbol in a way >that broke a module using it. > >-- thorpej ic, this probably explains why it's used sparingly so far. Thanks for the explanation. I'll try to educate myself on this wrt enabling this for modload. The other option for me is a bunch of ugly #ifdefs , which I'm trying to minimise. -- Sent from my Android device with K-9 Mail. Please excuse my brevity.
Re: Weak x86 aliases
On December 28, 2018 9:54:11 PM GMT+05:30, John Nemeth wrote: >On Dec 28, 7:36pm, "Cherry G.Mathew" wrote: >} Maxime Villard writes: >} > Le 28/12/2018 �� 14:57, Cherry G.Mathew a ��crit : >} >> Maxime Villard writes: >} Introduce a weak alias method of exporting different >implementations >} of the same API. >} >>> >} >>> Please revert or fix this change. >} >> >} >> I'm not sure what the fix is - do you have a suggestion ? >} > >} > either re-apply it without using weak symbols, or change the >modloader >} > to accept weak symbols >} >} I don't like the imperative in your tone. NVMM is the user of >modloader, >} not PVHVM. So if you feel like your usecase needs fixing, I'd say >it's >} your problem - or don't use modules, but see below. > > I suspect there's a language issue here due to people using >English as a second language. However, I don't see an imperative >(command) here. You asked for suggestions on how to fix a problem. >He answered your question with a couple of suggestions. That's >all. > > Also, I would argue that the kernel uses modloader, not the >module. In any event, as mentioned, it is your change that broke >things... > >}-- End of excerpt from "Cherry G.Mathew" On second reading, I take that back. Replied too hastily, and maxv has kindly offered to work around the problem temporarily. -- Sent from my Android device with K-9 Mail. Please excuse my brevity.
Re: Weak x86 aliases
On December 28, 2018 8:40:11 PM GMT+05:30, Jason Thorpe wrote: > >> On Dec 28, 2018, at 6:06 AM, Cherry G.Mathew wrote: >> >> I don't like the imperative in your tone. NVMM is the user of >modloader, >> not PVHVM. So if you feel like your usecase needs fixing, I'd say >it's >> your problem - or don't use modules, but see below. > >...but as the person who committed the change that broke it, you're >responsible. His use case didn't need fixing until you introduced weak >symbols that our kernel loader can't currently resolve. > >-- thorpej That seems like a dependency burden on people developing new features. I can't tell if modload is the only way to get NVMM to run. If it is, then ok. If not, then there's the option of provisionally turning it off until we fix the underlying problem. -- Sent from my Android device with K-9 Mail. Please excuse my brevity.
Re: Weak x86 aliases
On December 28, 2018 7:58:10 PM GMT+05:30, Maxime Villard wrote: >Le 28/12/2018 à 15:06, Cherry G.Mathew a écrit : >> Maxime Villard writes: >>> Le 28/12/2018 à 14:57, Cherry G.Mathew a écrit : Maxime Villard writes: >> Introduce a weak alias method of exporting different >implementations >> of the same API. > > Please revert or fix this change. I'm not sure what the fix is - do you have a suggestion ? >>> >>> either re-apply it without using weak symbols, or change the >modloader >>> to accept weak symbols >> >> I don't like the imperative in your tone. NVMM is the user of >modloader, >> not PVHVM. So if you feel like your usecase needs fixing, I'd say >it's >> your problem - or don't use modules, but see below. > >Wut? Yes my suggestions are either we re-apply the change without using >weak symbols or we change the modloader to accept weak symbols. > It's ok for the diff involving the specific function, but in general, we'll need option #2 because I can't see a less intrusive way to dynamic switching between native and PV. ... > >That's fine, I can hard-code the check on my side for now, to make it >load. Thanks, appreciate it. I'll have a look at the kobj thing. Cherry -- Sent from my Android device with K-9 Mail. Please excuse my brevity.
Re: Weak x86 aliases
Cherry G. Mathew writes: [...] > I think I'll revert these for now, PS: I'm in transit and probably not the best time to do this - if you feel like it's urgent enough, please feel free to rollback. I'll fix breakage on my end once the dust settles. I'll be in a commitable place in ~48hrs. -- ~cherry
Re: Weak x86 aliases
> On Dec 28, 2018, at 6:06 AM, Cherry G.Mathew wrote: > > I don't like the imperative in your tone. NVMM is the user of modloader, > not PVHVM. So if you feel like your usecase needs fixing, I'd say it's > your problem - or don't use modules, but see below. ...but as the person who committed the change that broke it, you're responsible. His use case didn't need fixing until you introduced weak symbols that our kernel loader can't currently resolve. -- thorpej
Re: Weak x86 aliases
Maxime Villard writes: > Le 28/12/2018 à 14:57, Cherry G.Mathew a écrit : >> Maxime Villard writes: Introduce a weak alias method of exporting different implementations of the same API. >>> >>> Please revert or fix this change. >> >> I'm not sure what the fix is - do you have a suggestion ? > > either re-apply it without using weak symbols, or change the modloader > to accept weak symbols > I don't like the imperative in your tone. NVMM is the user of modloader, not PVHVM. So if you feel like your usecase needs fixing, I'd say it's your problem - or don't use modules, but see below. >>> The kernel modules that use these functions can't be modloaded >>> anymore, because weak symbols aren't resolved. >>> >>> Eg, NVMM can't be modloaded anymore, because it uses rcr0 among others. >>> >> >> I think I'll revert these for now, because PVHVM doesn't/shouldn't use >> them anyway, but I'd like to know how to fix this properly. modload not >> working due to __weak_alias() sounds like something we don't do properly >> in the modload path. > > There is a check in kobj_sym_lookup(): > > 926 case STB_WEAK: > 927 kobj_error(ko, "weak symbols not supported"); > 928 return EINVAL; > > To resolve correctly I guess we need to take the strongest of the > duplicated symbols. > I'll look into this unless someone else beats me to it - but for now I'm ok for the specific change that breaks things for NVMM to be rolled back. (Please see other email about timeframe). -- ~cherry
Re: Weak x86 aliases
Le 28/12/2018 à 15:06, Cherry G.Mathew a écrit : Maxime Villard writes: Le 28/12/2018 à 14:57, Cherry G.Mathew a écrit : Maxime Villard writes: Introduce a weak alias method of exporting different implementations of the same API. Please revert or fix this change. I'm not sure what the fix is - do you have a suggestion ? either re-apply it without using weak symbols, or change the modloader to accept weak symbols I don't like the imperative in your tone. NVMM is the user of modloader, not PVHVM. So if you feel like your usecase needs fixing, I'd say it's your problem - or don't use modules, but see below. Wut? Yes my suggestions are either we re-apply the change without using weak symbols or we change the modloader to accept weak symbols. ? The kernel modules that use these functions can't be modloaded anymore, because weak symbols aren't resolved. Eg, NVMM can't be modloaded anymore, because it uses rcr0 among others. I think I'll revert these for now, because PVHVM doesn't/shouldn't use them anyway, but I'd like to know how to fix this properly. modload not working due to __weak_alias() sounds like something we don't do properly in the modload path. There is a check in kobj_sym_lookup(): 926 case STB_WEAK: 927 kobj_error(ko, "weak symbols not supported"); 928 return EINVAL; To resolve correctly I guess we need to take the strongest of the duplicated symbols. I'll look into this unless someone else beats me to it - but for now I'm ok for the specific change that breaks things for NVMM to be rolled back. (Please see other email about timeframe). That's fine, I can hard-code the check on my side for now, to make it load.
Re: Weak x86 aliases
Le 28/12/2018 à 14:57, Cherry G.Mathew a écrit : Maxime Villard writes: Introduce a weak alias method of exporting different implementations of the same API. Please revert or fix this change. I'm not sure what the fix is - do you have a suggestion ? either re-apply it without using weak symbols, or change the modloader to accept weak symbols The kernel modules that use these functions can't be modloaded anymore, because weak symbols aren't resolved. Eg, NVMM can't be modloaded anymore, because it uses rcr0 among others. I think I'll revert these for now, because PVHVM doesn't/shouldn't use them anyway, but I'd like to know how to fix this properly. modload not working due to __weak_alias() sounds like something we don't do properly in the modload path. There is a check in kobj_sym_lookup(): 926 case STB_WEAK: 927 kobj_error(ko, "weak symbols not supported"); 928 return EINVAL; To resolve correctly I guess we need to take the strongest of the duplicated symbols.
Re: Weak x86 aliases
Maxime Villard writes: >> Introduce a weak alias method of exporting different implementations >> of the same API. > > Please revert or fix this change. I'm not sure what the fix is - do you have a suggestion ? > The kernel modules that use these functions can't be modloaded > anymore, because weak symbols aren't resolved. > > Eg, NVMM can't be modloaded anymore, because it uses rcr0 among others. > I think I'll revert these for now, because PVHVM doesn't/shouldn't use them anyway, but I'd like to know how to fix this properly. modload not working due to __weak_alias() sounds like something we don't do properly in the modload path. -- ~cherry
Weak x86 aliases
Introduce a weak alias method of exporting different implementations of the same API. Please revert or fix this change. The kernel modules that use these functions can't be modloaded anymore, because weak symbols aren't resolved. Eg, NVMM can't be modloaded anymore, because it uses rcr0 among others.
