[Skiboot] [PATCH] hw/bt: allow BT driver to use different buffer size

[Cédric Le Goater]

On 01/11/2016 07:09 AM, Cyril Bur wrote:
> On Wed,  6 Jan 2016 10:57:13 +0100
> Cédric Le Goater <clg at fr.ibm.com> wrote:
> 
>> The buffer size used by the BT driver defaults to 64bytes even when
>> the BMC reports a different size. This patch removes the restriction
>> as there is no apparent reason for it.
>>
> 
> The only reason it was written like this was to avoid breaking anything, I like
> this change especially if we have things out there that can make use of bigger
> buffers.

Now that the IMPI patchset from Corey has been merged, you can try it out 
with qemu, which uses 255 bytes buffer size.  

If you feel adventurous, here is my dev branch which contains a merge of
Ben's patchset on top of qemu's HEAD an a serie of mine : 

	https://github.com/legoater/qemu/commits/powernv-ipmi

run as usual :

	qemu-system-ppc64 -m 2G -M powernv -device ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10 -nographic -nodefaults -monitor stdio -serial pty -snapshot  -S -bios skiboot.lid -kernel zImage.epapr -initrd rootfs.cpio.xz


skiboot should contain some enablement for qemu. Here is a patch :

	https://github.com/legoater/skiboot/commit/16c1317c8c95b8429ee82fbf3a6d3c9876e7208e

>> The extra byte added on the input and output buffer size in the test
>> also seems superfluous. The buffer lengths used later in the code are
>> correct as they take into account the extra byte needed for the
>> message size :
>>
>> 	bt.caps.input_buf_len = msg->data[1] + 1;
>> 	bt.caps.output_buf_len = msg->data[2] + 1;
>>
>>
>> From IPMI Specs:
>>
>>     22.10 Get BT Interface Capabilities Command
>>
>>     For Bytes 3 and 4 (Input and Output Buffer size), the buffer
>>     message size is the largest value allowed in first byte (length
>>     field) of any BT request or response message. For a send, this
>>     means if Get BT Interface Capabilities returns 255 in byte 3
>>     (input buffer size) the driver can actually write 256 bytes to the
>>     input buffer (adding one for the length byte (byte 1) that is sent
>>     in with the request.)
>>
>>
>> Signed-off-by: Cédric Le Goater <clg at fr.ibm.com>
>> ---
>>
>>  Tested on palmetto which uses a 64bytes fifo and qemu which uses
>>  255bytes.
>>  
>>  hw/bt.c |    6 ++----
>>  1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> Index: skiboot.git/hw/bt.c
>> ===================================================================
>> --- skiboot.git.orig/hw/bt.c
>> +++ skiboot.git/hw/bt.c
>> @@ -164,16 +164,14 @@ static void get_bt_caps_complete(struct
>>  		goto out;
>>  	}
>>  
>> -	if (msg->data[1] + 1 != BT_FIFO_LEN) {
>> +	if (msg->data[1] != BT_FIFO_LEN) {
> 
> The aspeed2400 doc (I can't seem to find where I got it from... sorry) states
> that the hardware is 64 bytes so i would expect the BMC to return 63 here?
> Admittedly I never actually checked what the BMC returns, have you observed
> something different from BMC firmware? 

We get 64 bytes from the BMC (palmetto and habanero). Here are 
the logs with this patch :

[1055047556,7] BT: seq 0x00 netfn 0x18 cmd 0x36: IPMI MSG done
[1055050249,7] BT: BMC BT capabilities received:
[1055051990,7] BT: buffer sizes: 65 input 65 output
[1055054016,7] BT: number of requests: 2
[1055055689,7] BT: msg timeout: 2 max retries: 1



>>  		BT_DBG("Got a input buffer len (%u) cap which differs from the default\n",
>>  				msg->data[1]);
>> -		goto out;
>>  	}
>>  
>> -	if (msg->data[2] + 1 != BT_FIFO_LEN) {
>> +	if (msg->data[2] != BT_FIFO_LEN) {
>>  		BT_DBG("Got a output buffer len (%u) cap which differs from the default\n",
>>  				msg->data[2]);
>> -		goto out;
>>  	}
>>  
>>  	/*
>>
>> _______________________________________________
>> Skiboot mailing list
>> Skiboot at lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/skiboot
>