 Okay, everyone. Thank you for coming. Here is my presentation about something when I discovered recently a specification update, which as you will see soon is pretty minor, but causes some things. And as I was trying to find out how to do things and had to gather this information quite a lot, so I thought, let's just talk about it so other people will not have so much issues with that, hopefully. And not everything I concluded at the end said and so on. So if we, maybe we can have some discussion here or later, we will see. I'd like to get a first short overview of how many of you are familiar with I2C or SMBUS terminology. A few, not everyone. Okay, so I will just explain a lot. So let's start first with the look at the specifications, how it all started. I2C and SMBUS are super old, so I think I2C started in the 70s, and the specification I'm now looking at, the SMBUS specification is from 1998, and this has been working well so far, and this is kind of proven technology. And one thing which is in the standard is that you can transfer blocks of data and they introduced an upper limit for that. So if you read below the, no, the red, if you see the red byte count up here, it's basically somewhere in the stream of data you have a count saying so many bytes will be transferred and the data follows. And regarding the byte count, it is clearly said it may not be zero and it is allowed to transfer a maximum size, a maximum of 32 data bytes. That's pretty clear. Most people adhere to that and so things were working pretty well. So before the data we send the length of the data block. It's basically the same for reading, so before you get the block of data you get a byte saying how much data is to be transferred. The special thing which is a bit unique in the I2C and SMBUS world is that this information comes from the client, comes from the target. Usually the bus controller is deciding everything on the I2C bus also and so in I2C transfers also usually the length, but here we get the data from the target saying A, I'm going to send you that many bytes, which according to the specification should not be longer than 32 bytes. We will come back to this later because drivers need to take care of that. They first need to read the first byte and then they know what's going to happen. This has some implications with the buffers involved. And then in 2014, I call this new, of course it's not really new anymore, there was a slight update, it was I think a bit hidden and now says the byte count may be zero and the block read or block write is allowed to transfer a maximum of 255 data bytes, so the full 8-bit range. There is no nothing like versioning update, it's the same data call, it's just that the upper limit was raised. And when I say what does it mean that the byte count may be zero, I think the first idea you get is, well, then there's no data to be transferred, but it's not clearly stated. I bet that some creator of some target or some device will say 255 is such an odd value, I want to transfer 256 bytes, so I'll just say zero is 256. I'll bet such a device will appear and if you read the standard very strictly, it's not forbidden because it may be zero, what does it mean? So I think this is a little bit of a flaw in the specifications, but I think we should prepare for that. So this is all what initiated the next slides. I will just make a small interlude because when preparing this talk I found out there's a new version of the SMBUS specification released in January this year and they fixed some minor details or improved the paragraphs for description, but the main, by far the main change was improvement in language. Throughout the document, master was replaced by controller and slave was replaced by target. And I was quite amazed to see that happen because SMBUS is an organization so a lot of companies had to agree this one and I think it's good because there have been people trying to improve the language also within Linux and I have sympathy for that. I mean you could look technically on that, yeah it's a slave, it doesn't have any to say and the master is controlling the bus, so I can see where that comes from, but slavery had really some, it's one of the cruelest things we had so if you meet someone and say hey your grandfather was a slave, here look my device is a slave too, that doesn't really fit. So I'm all for improving that language and now having an official documentation defining a better wording by saying controller and target I think we can now do that in Linux as well and use these terms and as it's always also said in the specification this was an extensive change to the text files and figures, it will be the same for Linux. If somebody wants to help me with that I'm all for it because now we can point to some specification and say okay, that's a new terms. So I was happy about it. That was also a good change in the specification. Now let's go back to the size limit, it used to be 32 bytes, now it's 255 and what it was introduced in 2002 is not very surprising, it's a simple define, 32 bytes is SM bus block, maximum max. Before that the limit also exists, it was just hard coded so this was changed for the better in 2531, we now have a define for that. Luckily it's not so widely used so if you want to check the kernel tree for that you have 200 uses of that so if you need to audit, 200 is not exactly nothing but it's not so overwhelming that it can't be done so that's a good thing. So can you read that there, nice. Now let's have a look how it's handled within Linux. Let's assume you want to send a block of data that I described before. Where is my laser pointer, there it is. Then you have some struct which defines a target where it should go to, the command we can ignore that's just one byte describing the command but here it gets interesting, here's the length and it was also said at most 32 bytes and then you have just a byte array, pointer to byte array which get passed and it's the length defining that array that's what you pass, we're now inside the Linux kernel so if you want to use this function in the Linux kernel that's how it looks and what the function does, it first takes a union that was the first time I got a little bit of shiver I mean they're not bad per se but usually if you encounter unions you send some problems coming ahead and then the upper limit from the spec is enforced if the length you provided is bigger than this maximum then it will be cut off at that length. I think it should have returned an error but this is an behavior which is in Linux since 1998 so we're not going to change that I have no idea how many applications are depending on that or even in kernel code we're here inside the kernel space and once the check is done the length is prepended to some data block the values you provided are appended using a mem copy and all this is then put to a generic transfer function and this generic transfer function you see here it gets this data union buffer something and the only way to know which size the buffer is is encoded here in this, it's another define it says what I want to do is a block data transfer and this has, as we know from the specification in upper limit of 32 bytes that's how it works, it works quite well and if you see what the union is okay we have it because I just said we have this block data transfers there are other transfers like a byte transfer like a word transfer and to handle all these data types back then they thought union was a good idea so we have this union which can either be a byte, a word or a block which is statically so it's not a pointer to some memory it's the memory itself and it has this size as we have seen block 0 is used for the length it got encapsulated for the length and the data and then you can see and one more for user space compatibility so there had been another format in the past which had been considered bad but in order to not break user space we still have to keep the size the same otherwise if you would have an array of this the size would not match the compiled binary from the user space would have a different size than the compiled binary in the kernel and they will not get the proper positions inside that array and we'll get everything wrong so we have to live with that extra byte no that didn't work so this does work in user space I thought we were in kernel space but if you look here that's the function this is the library it's basically the same you have the union you check for the length this is an open coded mem copy I don't know why then you have the block length at the beginning and you pass it to a generic smbus transfer function and again you have here the buffer and the only indication what is the size of the buffer is this type of smbus transfer that's the only indication we have as I mentioned before there are other transfer types indicating the size of this union it's just a set of defines here that's the one we're looking at if you look closely you have I2C block broken so there has been another try in the past to get it right which was being considered not so good so this is there and we have to live with it everything maybe you noticed here the path says UAPI so everything you see here is exported to user space that means the user space and the kernel have to agree on these numbers it's by definition 5 means I1 the size of the buffer is suitable for our block data transfer if you use another number the kernel will get confused and if you would just remove this one and make block proc call a 6 then it won't match anymore and you will have lots of undesired results so we're exporting this to user space this has to stay sadly another occasion where this maximum is used is as I said before there is this read transfer where the data byte comes from the target and tells you how many bytes it wants to send so that means each and every driver has to do supporting that transfer type has to do basically like this if the first byte comes then it has to check if it's either 0 or bigger than that and return a protocol failure if so and if not that in runtime while processing the transfer itself the length gets updated by whatever the target returns so because you get data from the target you really have to pay attention not to run into buffer overruns here and as you can imagine with every limit you impose there's a high risk but there is a risk of off by one errors and some drivers forget to check for the length equals 0 check and some are confused and think the length byte belongs to it and check for blockmax plus one so it's really annoying but given the current the old way of the standards this is how it works I discovered this update of the specs I think I mentioned it was in 2014 I discovered it in 2015 and thought let's update it so pretty naively and figured soon as it was you can't do it by and I didn't have any hardware what I did someone later there's a test unit that's a piece of code where utilising that linux can also be an i-squashy target and then can simulate basically anything and I wrote some code to simulate this as emboss block behaviour so assuming my implementation was right I had at least some hardware hoping to have a check for this new limit but actually doing it I didn't want to do it alone and I didn't really have the time to do it as a side task and then 20-20, 6 years later the first user who wanted to have it appeared on the mailing list who had a PMBus device which is another subset of SMBus and I didn't know they have certification, they don't have the 32 byte limit I never knew that and he wanted to improve access and wrote even two RSEs how to do it but since the issue was so big he just found another way which is very sad because he was for the time he had he didn't have so much time to work on it but the time he had he was very eager to get it right and a bit later Sultan Alzavov I hope I pronounced his name correctly came and he had a surprising information for me he was only interested in fixing it in one driver for his laptop because he said if he extends the length of the transfer and reduces the number of transfer he could save 1.6 watts from his laptop usage and I said well that's a lot and he even claimed that without his patch improving the situation his touchpad would get really warm without it it would be okay and so his works for me approach was all certainly not upstreamable but the information is that wow that makes a difference was new and helpful for me and in 22 there was February this year there was the last approach from Matt Johnson who wanted to encapsulate some other over I square C and of course if you want to have a stream transferred 255 bytes are a lot better than 32 but also given so he tried a little but given his time constraints he just went with the old limit so let's see ah there the counter is working so possible solutions the most naive approaches yeah well just increase the limit it's a one line patch and what would happen then I mentioned this before it wouldn't work OK with some bit of audit inside the kernel because inside the kernel we have control over all users of this define and so we can make sure it always fits but as I said before it's exported to user space and you have the problem that you have a user space which is compiled against old kernel old kernel headers meaning in this case 32 bytes and the kernel is with new its own headers 255 bytes so now imagine an old user space saying hey give me I want to read a block of data here's your 32 byte buffers buffer and the kernel says oh cool there's a buffer it's 255 bytes yay I'll fill it OK crash boom whatever goes wrong this is not good cannot do this so what Daniel Stodden proposed was to keep the old define as it is and define a new one and use it here in the union so whenever we encapsulate the data we make sure it's big enough we don't know if we use it but we make sure it's big enough this is the one part but remember there was these defines which describe the buffer how big it is these you know maybe you recall them these were the old ones block data and of course now if you change the size of the buffer you need new new defines to describe how big is a buffer and what he did where is my laser here it is he renamed the legacy ones to from SM bus to SM bus one but kept the number and introduced new ones with higher numbers so he gets some compatibility if there's an don't mix it up if you have an old user space using the old limit it will still send the five and the kernels looks up five oh it's still 32 bytes and it will work if you get if you recompile your application without changing anything you will get the new limit for free because that's that's the old name SM bus block data it used to be the old name it will just send a nine instead of a five to the kernel and then this kernel say nine hey you have a bigger buffer no problem here's your bigger block of data big problem what if you have a new user space and an old kernel the new user space says hey I want a huge block of data here number nine describes it and the old caller says number nine never heard of that failure can't do you see if you have an old kernel user space new kernel user space will always match no problem it's always this old versus new problem so the next thing you could think of okay then we leave these numbers completely alone and just introduce new numbers for the bigger transfer size opt in we're getting better but it then we have still a problem with libraries and in a perfect world this doesn't need to go wrong but if you think about ABI's you should really believe in Murphy's law everything which can go wrong will go wrong let's assume you have a shiny new where's my laser point a shiny new kernel and in which supports the new block size and your application is self compiled and you use the new kernel headers you have also the new block size but you're running Debian for example and use lib I2C and your user land is still compiled against some older kernel headers which only know 32 bytes then your application with you this will use this block write function with the new limit 255 bytes and as you have seen in user space there's a check if your length is bigger as m bus block max it will cut it to that limit for the library the limit is still 32 bytes so it will just send 32 bytes to the kernel and you wonder why things are not working the way as you'd expect them and until finding out that this is a library which compiles again kernel headers that will take some time I guess so I'm not I'm not really favoring this option but we're getting closer to that I think we really need to use a new IOCT alcohol make it completely opt in. Don't touch anything which has ever been exposed to user space so we do define the new maximum we do define a completely new type which has the bigger limit and this type is only used with a special IOCT alcohol which is new this is the old one we will never ever touch it we will just introduce a new one and this makes also this old versus new a bit better because if you call into the kernel and say hey I want to use this new IOCTL and the old kernel doesn't know about it it will just say it gives a proper error message it doesn't cut off your buffer it just gives you a proper error message so applications can try to use the new IOCTL and if they get a failure you can still use the old one so far I think I haven't found yet a scenario where this should not work you have to update your applications you have to update your libraries yes but regarding regressions this is kind of good I think and what I also I'd still change it a little you see how much pain it is to change something in all this ABI stuff so I would just use the block maximum as 256 and not 255 because I'm quite sure such a device not adhering to the specs will appear I mean that's my experience as a better developer for oh gosh for a long time now it needs of course a command saying okay yeah we know the standard is like this but for reasons we add one to it and to make this accessible to function to make this distinguishable we need one bit which can be set so you know your device supports that or not then you can decide I will do this or that hooray something easy once all this done we can have one bit defined and let applications do their thing so although I've been talking quite some things it's not all I have covered but otherwise I'll be talked in an hour longer there are some other cases to be considered there's another transfer type called block process call which needs to be considered as well there is an IOCTL called read write where you can send custom messages that also needs to be considered if you use this receive length I mentioned before but the basic ideas and the basic problems are the same so if we got a good approach with this block write and block read we can transfer that to the others and just need to do a little bit more of audit but that's doable it should work and we should not regress that for sure so we need to pay attention but the use of these features is not so high so we don't need to audit so much so okay and I mean iSquashy is not a super sexy band with high developer subsystem so is it really worth doing all this and I agree I think yes because I think it would be good for linux to say hey we're having full or better compatibility for at least as mbus3 and pmbus I mean this is used in servers quite a lot and there are also some clients who actually need this functionality with most you can fall back to the smaller transfer size okay but some new devices you cannot do this you need the huge transfer size otherwise they won't work at all I also think the power savings which were new to me are really good idea to have I mean if the touchpad saves power it's good if you have less cpo utilization it's also good and I think we also have better drivers bus controller drivers because it allows us to remove these error prone checks from the drivers they can just go away because we with a new size limit we have always buffer which will fit so this annoying code can go away what is not are the free support smbus with the second last line I mean smbus is a subset of iSquashy so if you have a regular iSquashy controller it will be able to handle the new limit automatically it's not like we need to wait for new hardware to actually support that new feature most hardware out there will support it if we get the buffers right and this is what I mean for free we get we're not so hardware dependent with that what it will not be it will not be much faster I mean you save some execution time on the cpu but well we're talking about the bus here working on 100 or 400 kilohertz and you still have to do all the transfer speed in one chunk or multiple chunks the overhead when it comes to execution times it's not so big compared to that but the utilization is still nice because you might go to if you can offload the data transfer you might go into a deeper sleeping state that is also good and the path forward as I see it I hope it became somewhat obvious by now in user space keep everything existing as is and everything else is opt in make the new iocgl make the new define for block size and don't touch anything else kernel size is a little bit on the kernel space it's a little bit different because I really would like to make the new size the default for the buffers so we'd never ever run into buffer overflows I have really again Murphy's law if we have two kinds of buffers people will mix it up and I will just make sure that we have one buffer size which is big enough to never overrun luckily for us these buffers I only used mostly once at a time it's not like we're using tons of memory basically we're losing 20 bytes or something and I think for the improved safety this is worth it and I also allowed it if we have the buffers which are always big enough then we move we can move checks from the bus controller drivers which just try to enforce a protocol to the client drivers which actually what data kind of data they will expect when they expect and if the size is proper despite the upper limit I mean the client knows I want a string of 10 bytes and everything else then 10 is wrong regardless of the limit of the transfer so I think we should do it like this so my conclusion for dealing with this case and I think it will fit for most case if you want to change user space opt-in is the way to go don't try to modify something which is already existing because of these old user space versus new kernel or new user space versus old kernel mismatch lots of things can go wrong divide and concord still helps a lot it really helped me to tackle the problem better or understand the problem better if I was separating it between kernel and user space and this is not this is really a big task and as I said, i-squash-e is not the network layer if I'm quite sure if you could somehow decrease the latency of the network layer a lot more people would jump on the wagon to help doing that but for i-squash-e I was except for the 3-4 people I just mentioned basically alone with it but I need some focus and you can do a lot of things wrong so that's why I need some focus on that and currently I didn't have that because I have other jobs to do so that's why we still after 8 years still not have it although we now have a roadmap I have to thank renaissance for partly funding my work so they're contracting me and allowing me to do i-squash-e work on the site and we will not be here if they didn't do any kind of support for me but implementing all this is another task which I try to get funded somehow I can't really put all this work only to them because they also have other interests but yeah at least they did this to so I can present a roadmap to you and with only 2 minutes left on the clock this was my talk about user space abis I hope you got some idea what is involved in that and if you're sometimes in the talk got the feeling ooh here be dragons then I did something right because there are dragons thank you very much are there questions oh yeah there's one can you please go to the microphone if you are adding a neo kernel api anyway why not do away with the union drop that u8 at the beginning and like add a new member for the lens so in the future if you happen to need more than 265 bytes the user space can just say my buffer is that long why not use the occasion for that so like have a lens value struck instead of the union I see what you're saying so you maybe make a pointer out of it instead of another fixed array size buffer something like this could be argued I think there's one argument against it the specification defines this block right especially with one byte of length if they would introduce 2 byte of length they really have to make a different data type out of it and then we would completely need to handle a different anyhow so I can I think I can safely say 8 byte is the upper limit for that transaction type but still you could remove that unions one of one or two of these union members for the new kind of pointless to have a u8 byte and then having the same union a u8 byte array you can just that could be that during implementation that comes that makes sense there's another question could you please come to the microphone and this will be oh no it's lunch afterwards so we can go on forever is there a way to deprecate the old one or will it leave forever is there a way to detect what? deprecate, remove this struct and the data length deprecate there's SMB and SMB in kernel space or in user space both maybe so actually I'd like to deprecate it in kernel space to make sure like as I said every buffer has a bigger size which cannot overflow in user space you can't I wouldn't really do it because you need to have it anyhow for for backwards compatibility and I think it's okay if you know that this block data you want to receive is below 32 bytes then why not? this may not be, I suppose your SMB may not be getting too many updates but in a case it does won't it keep get longer, bigger and bigger you will just keep adding new CTLs, data structures if there's no proper way to deprecate old ones is the problem? deprecating as in hey please don't use it anymore switch to another one, yes removing, no thank you any question left? good then let's go to lunch thank you everyone